Re: [spec-reviews] requestIdleCallback (#70)

Replying inline to @dbaron's comments 
> Does the spec's notion of "idle" scale well to a more multi-core future in which browsers do a good job of taking advantage of those cores? In particular, engines may be moving to doing more tasks (e.g., beyond compositing to tasks that are more part of layout or even style computation) on other threads, or (probably less worrysome here) dividing a main-thread-blocking task across multiple threads to help it finish faster. I tend to think the APIs are probably ok in that regard, given that they're a relatively straightforward extension of setTimeout, but I'm a little more worried about the prose (both the diagrams of how things work in section 2 and the processing model in section 5 -- see the next point).

I don't think making things multi-core changes the concept of idle tasks - browsers will (rightly) continue to move more work off the main thread, however there is likely to remain a decent proportion of work which needs to happen on the main thread for legacy reasons. The diagrams (e.g., section 2) are purely for illustration and aren't meant to constrain where idle periods can or should be started by the browser (I'm open to someone changing these diagrams to be more generic, however it is explicitly in a non-formative part of the spec, so I'm not sure if that's necessary).

> Perhaps a slightly better concrete example for this concern: suppose that an implementation wants some work that doesn't block the main thread's event loop to still make things count as "not idle", so that the idle callback work doesn't detract from that work happening on other threads.  One could interpret the first paragraph of the processing model section of the spec as disallowing such consideration ("assesses that a given event loop is likely to remain idle for a non-trivial amount of time"), but I think considering such things should be allowed.

We could change this line to "assesses that a given event loop is likely to remain idle for a non-trivial amount of time, and that background work could be executed on this event loop without impacting any high priority work occurring on other event-loops" - would this help?

> 2. I'm worried the processing model is over-defined.  (This concern comes from bzbarsky, who I believe raised an issue on it; I think w3c/requestidlecallback#28 although it might be any one of 25-29, and I don't have good enough internet access to check right now.)  For example, it defines the duration of an idle period at the start of that idle period (steps 4, 5, and 8 of the "When the user agent wishes to start an event loop's idle period" algorithm), and earlier says that no other idle period can begin before that deadline has passed ("Let last_deadline be the last idle period deadline associated with the current event loop, or 0 if no previous deadline exists."; step 1 of the same algorithm), so essentially it forbids running any idle callback that wasn't posted before that deadline was determined until after the deadline.  This seems like it might lead to underutilization of perfectly-usable idle time; I'd rather implementations be allowed to use the idle time effectively by 
 calling idle callbacks, using strategies that they deem best for speed, power usage, interactivity, etc.  (There might also be good reasons to want such usage, e.g., because it may be better to do a bunch of work together than to wait and cause additional CPU wakeups.)

The reasoning behind this is discussed in more detail on issues w3c/requestidlecallback#27 and w3c/requestidlecallback#28 (perhaps we could followup there to keep the discussion in one place) - essentially this is necessary to allow the programming model where you check if you have enough idle time to do anything and reschedule if not.

However, the API could be extended with additional scheduling models via the `IdleRequestOptions` dictionary. We could add something like a `callback_type` property to `IdleRequestOptions` which would allow developers to specify that the callback should be run immediately (i.e., within the same idle period) if possible, while still retaining the current scheduling model as the default mode.

> That said, I'm not against all constraints in the processing model; a number of the other constraints (e.g., relating to the relative order of idle callbacks, or the fact that each idle callback is called from a separate queued task posted after the previous one finishes) seem good.  (This applies to both the normative description of the processing model in section 5 and to its non-normative description in section 2.)  For example, if an implementation ends an idle period whose deadline was initially 50ms long only 10ms into it because there was user input, but then finishes processing that input 10ms after that, the spec currently requires not starting another idle period for another 30ms.

Yes, this is an example of things being a bit over-constrained, although there are reasons why we need to limit when idle periods can start as described in https://github.com/w3c/requestidlecallback/issues/28#issuecomment-148422630, along with a potential approach to reduce these constraints somewhat.

> On the flip side, there are also downsides to some of the other constraints:  for example, they don't allow implementations all that much leeway to prioritize a visible tab over a hidden tab (presuming they share an event loop) within a single idle period, beyond allowing the visible tab to have its first idle callback executed before the hidden tabs' first idle callback, and limiting all of the hidden tabs to exactly one idle callback by terminating the repeating (which would then be wasteful if the visible tab didn't have enough work to finish the idle period).

The processing model currently doesn't specify what should happen for hidden tabs (at least as far as I read it, could you point at the section which allows them to run once then terminates their repeating?). However, the intention was to be closer to what you describe, where visible tabs can be prioritized over hidden tabs (i.e., the first note in the Processing Model section). I agree that we should probably make this more specific in the spec, I'll try to do so as part of w3c/requestidlecallback#31.

> I'm also wondering if it's less than ideal that there's one "invoke idle callbacks algorithm" _per document_ as a queued task during the idle period.  I was thinking that the queueing of a separate task for each callback meant that tasks queued from a callback run before the next idle callback (which is probably good for memory access locality and thus performance, if the tasks are small, at least assuming implementations don't blow all CPU caches just by queueing and executing a task).  But now I realize (I think) that that isn't true; tasks queued from an idle callback run after the next idle callback for all the other documents, but before the next idle callback for the same document.

This is an artifact of trying to allow inter-document reordering but avoid intra-document reordering. We could probably have a single queued task which selected the next idle task based on posting order (regardless of which document it came from) if you are happy to impose that inter-document idle tasks are also FIFO ordered (also see comment on inter-document FIFO order below).

> #### Detailed comments
>
> I also have a few more specific comments from reading the spec.
> 
> ##### 1. Introduction
> 
> Do you really want to mention localStorage as an example, given that we'd like to discourage use of it (in favor of asynchronous storage APIs like IndexedDB)?

I don't mind removing this - I will do this as part of w3c/requestidlecallback#31.

> "can't make any assumptions" should probably say something more like "can't make accurate/good/??? assumptions" (i.e., it can make assumptions -- they will just often be wrong)

Sounds good to me - I will do this as part of w3c/requestidlecallback#31.
 
> ##### 2. Idle Periods
> 
> First paragraph seems to imply that input processing is for a given frame, which seems a little fishy to me.  (Or maybe that's true in some browsers?)

In Chrome we batch-up input operations and perform them once per frame. I'm open to changing this to make it more generic if you have a suggestion.

> "The user should be careful to account for" doesn't mean the user of the browser (which is what we usually mean by "user"); it means the user of the API.

I'll change this to be 'web developer' as part of w3c/requestidlecallback#31.

> This says the callbacks are run in FIFO order, but it's really only FIFO for each document separately.  (Though maybe the inter-document order should be better-defined, and defined in a way that doesn't allow one document's idle callbacks to starve another document's, which seems possible in the current model?)

In Chrome we do constrain inter-document order to be FIFO too, so I don't mind changing the spec to be more constrained in this regard, I just wanted to keep things as unconstrained as possible for other browsers.

> "Once an idle period is finished the user agent can schedule another idle period if it remains idle" isn't strictly true per the processing model (given that the UA can't schedule another idle period until the previous one's deadline has passed), although I propose above that that be fixed.

Discussed above - once we reach consensus on this I'll clarify this sentence.

> ##### 4. Window interface extensions
> 
> One possible point of confusion:  when I first read through the IdleDeadline interface, I thought "didTimeout" was an attribute that reflected whether timeRemaining() was greater than 0 or not (although I was thinking that this should be a function rather than an attribute).  I wonder if developers might have the same confusion, and whether there's a better name available.

Acknowledge, although I can't think of a better name myself.

> Use of "unsigned long" indices is a little bit clunky, although I guess it's compatible with setTimeout.  One thing I worry about is that people might be converting code from setTimeout to requestIdleCallback (or using requestIdleCallback if available and falling back to setTimeout) -- so I wonder if it might help avoid hard-to-debug mistakes from that conversion if setTimeout and requestIdleCallback used the *same* space of integers rather than overlapping spaces (and maybe even if clearTimeout and clearIdleCallback were just the same underlying implementation, or maybe even not introducing a new clearing method).

This was done to be compatible with setTimeout, etc.

> "Wait until any invocations of this algorithm started before this one whose timeout is equal to or less than this one's have completed.":  This seems like it might be a little bit underconstrained; it seems like it might be worth guaranteeing sorting by (requestIdleCallback time + timeout).

This was the intention, I'll try and clarify this a bit better as part of w3c/requestidlecallback#31.

> ##### 5. Processing model
> 
> "If the Document 's hidden attribute [page-visibility] is false" seems like it should say true rather than false.

Opps, yes I'll fix this as part of w3c/requestidlecallback#31.

---
Reply to this email directly or view it on GitHub:
https://github.com/w3ctag/spec-reviews/issues/70#issuecomment-151906699