- From: David Baron <notifications@github.com>
- Date: Thu, 22 Oct 2015 20:58:28 -0700
- To: w3ctag/spec-reviews <spec-reviews@noreply.github.com>
- Message-ID: <w3ctag/spec-reviews/issues/70/150457356@github.com>
Let me re-paste that "Further review of the spec" from two comments back, now that I realize that github doesn't accept markdown in email input but only in the Web UI. Note that the previous two comments are in reply to this one: #### Larger issues I have two larger issues to raise (although I admit the first is somewhat vague): 1. 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). 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 https://github.com/w3c/requestidlecallback/issues/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 i dle 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.) 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. #### 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)? "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) ##### 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?) "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. 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?) "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. ##### 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. 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). "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). ##### 5. Processing model "If the Document 's hidden attribute [page-visibility] is false" seems like it should say true rather than false. --- Reply to this email directly or view it on GitHub: https://github.com/w3ctag/spec-reviews/issues/70#issuecomment-150457356
Received on Friday, 23 October 2015 03:59:26 UTC