Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"deadline" in the "start an idle period algorithm" should be defined in terms of event loop #71

Open
rniwa opened this issue Nov 7, 2017 · 28 comments

Comments

@rniwa
Copy link

commented Nov 7, 2017

Right now, The start an idle period algorithm defines the deadline as:

Let deadline be a time in the future until which the browser expects to remain idle. The user agent SHOULD choose deadline to ensure that no time-critical tasks will be delayed even if a callback runs for the whole time period from now to deadline. As such, it should be set to the minimum of: the closest timeout in the list of active timers as set via setTimeout and setInterval; the scheduled runtime for pending animation callbacks posted via requestAnimationFrame; pending internal timeouts such as deadlines to start rendering the next frame, process audio or any other internal task the user agent deems important.

However, the list of active timers is defined in terms of a global object. I don't think that's the intent here. It needs to be defined in terms of the event loop. Perhaps we need a list of active timers for the entire event loop to be defined in the HTML specifications itself.

@rniwa

This comment has been minimized.

Copy link
Author

commented Nov 7, 2017

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Nov 10, 2017

I think this is best resolved by hooking into the HTML event loop spec as is being discussed in #70

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2018

So seems like this is blocked on #70

@yoavweiss yoavweiss added this to the Level 1 milestone Oct 17, 2018

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

@rmcilroy Would landing whatwg/html#4104 fix this issue as well? Or is there extra work to be done here?

@rniwa

This comment has been minimized.

Copy link
Author

commented Oct 25, 2018

No, we still need to define what it means for the event loop to be idle. The issue I raised is that just because this particular global object doesn't have any active timers, it doesn't mean other global objects such as those of iframe don't have a bunch of active timer soon to be fired. So this deadline needs to be computed the same way whatwg/html#4104 defines the timing at which this algorithm is invoked.

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

Right, this would need more work. It's not entirely clear how we could spec this - the current spec for setTimeout / setInterval relies on "wait for timeout milliseconds in parallel" before queuing a task on the task queue, so there isn't actually anything queued anywhere while the timer is pending that we could surface to the rIC algorithm. Any thoughts?

@domenic

This comment has been minimized.

Copy link

commented Oct 25, 2018

How is it implemented? Can we use that as inspiration for the spec?

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

In Chrome the delayed tasks are posted on a task queue while they are pending, ordered by their timeout/delay (this includes internal tasks as well as setTimeout tasks). We set the deadline to be the time of the earliest task in the queue (and also cap to 50ms or next frame deadline if we are rendering).

@farre

This comment has been minimized.

Copy link

commented Oct 29, 2018

This is pretty much how we do it in Gecko as well.

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Oct 30, 2018

It seems like modifying the HTML spec to accommodate this would be a decently big effort, and actually thinking about it I'm not sure how much it helps given the spec also wants to let the UA shorten the deadline earlier for internal tasks as well as those posted via the HTML spec, and to use heuristics to predict the likelyhood of an external event happening during that time. I.e.:

pending internal timeouts such as deadlines to start rendering the next frame, process audio or any other internal task the user agent deems important.

How about moving the discussion of the setTimeout, etc. to a non-normative note, and make the normative section explicitly regarding the event loop, i.e.:

Let deadline be a time in the future until which the UA expects the window's event loop to remain idle. The user agent SHOULD choose deadline to ensure that no time-critical tasks will be delayed even if a callback runs for the whole time period from now to deadline.

Note: Typically the deadline should be set to the minimum of: the closest timeout for the list of active timers as set via setTimeout and setInterval to the window's event loop; the scheduled runtime for pending animation callbacks posted via requestAnimationFrame; pending internal timeouts such as deadlines to start rendering the next frame, process audio or any other internal task the user agent deems important.

WDYT?

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2018

@rniwa friendly ping! :)

@rniwa

This comment has been minimized.

Copy link
Author

commented Nov 6, 2018

Again, my main issue with the spec is that this isn't precisely defined. Adding a non-normative note wouldn't cut it. In general, if the spec doesn't meet the bar of specifying a feature to the extent it should, then the right call is to delay the publishing of the spec instead of hand-waving the definition of it. We can't spec a deadline by enumerating examples. That precisely is the current spec's wording I have an issue with.

And, again, "given browsing context's event loop" doesn't make sense because the event loop is a global concept shared across a unit of related browsing contexts.

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2018

Fair enough, would someone be willing to add the necessary hooks into the HTML spec to make this happen? I don't have the bandwidth or experience with the HTML spec to do that at present.

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Dec 24, 2018

the spec also wants to let the UA shorten the deadline earlier for internal tasks as well as those posted via the HTML spec, and to use heuristics to predict the likelyhood of an external event happening during that time. I.e.:

pending internal timeouts such as deadlines to start rendering the next frame, process audio or any other internal task the user agent deems important.

Going back to this, and since the definition of "deadline" should also include heuristics regarding external events (which we don't intend to specify), I'm not sure what the benefits of specifying that would be.

@rniwa - are there any compatibility issues that you think will arise from keeping this value UA defined?

@rniwa

This comment has been minimized.

Copy link
Author

commented Dec 24, 2018

@rniwa - are there any compatibility issues that you think will arise from keeping this value UA defined?

Obviously. You can have a timer inside an iframe or a window then the deadline according to the current spec would be ignoring that. It's true that we can observe what Blink and Gecko had implemented and not make such a stupid mistake but the purpose of the spec is to clarify what needs to be implemented even in absence of any implementation. If we're forced to reverse engineer what Blink and Gecko did to implement this in WebKit, what the heck is the point of having this spec at all?

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2019

I believe the case of same-origin iframe or window are covered by the suggested normative language @rmcilroy suggested. Specifically, "Let deadline be a time in the future until which the UA expects the window's event loop to remain idle. " covers that case, as same-origin iframe or window share an event loop with the window.

Otherwise, I don't think that there will be benefits to defining UA-specific, internal events as part of the spec and it's better to keep them vague, to enable better flexibility when it comes to such heuristics.

That leaves us with the issue of timers. Timers do not currently queue tasks immediately, but only after the timer expired. They do register themselves on the "list of available timers", but that list is separate per global scope, even if multiple global scopes share an event loop.

So, maybe we can extend the suggested normative language and replace the current reference to timers of "the closest timeout in the list of active timers" with "the closest timeout in the list of active timers in any global scope that queues tasks on window's event loop".

Thoughts?

@rniwa

This comment has been minimized.

Copy link
Author

commented Jan 11, 2019

"Let deadline be a time in the future until which the UA expects the window's event loop to remain idle. "

That wording would work if we can define what it means for an event loop to remain idle.

Let's consider the following clause in the event loop processing model which defines when to start an idle period algorithm:

If this is a browsing context event loop (as opposed to a worker event loop), and there are no tasks in the event loop's task queues which are associated with a Document that is fully active, and the event loop's microtask queue is empty, and none of the browsing contexts have a rendering opportunity, then for each browsing context, run the steps in the start an idle period algorithm, passing the Window associated with that browsing context.

This definition makes it crystal clear as to exactly what condition constitutes the start of an idle period and when the algorithm is invoked relative to all other things happening in the browser. We need something similar. We need exactly what remain idle means. Is it that we don't expect a task/microtask to be scheduled in the current event loop? Is it that the rendering opportunity won't be happening? etc...

As for timers, yeah, we probably need to define the time at which the next/closest timeout from the list of active timers of each document of the browsing contexts which use the same event loop as a given window's browsing context.

@rniwa

This comment has been minimized.

Copy link
Author

commented Jan 31, 2019

Here is a list of things we discussed during F2F to explicitly list instead of "expect to remain idle"

  • The minimum firing time amongst the list of active timers
  • The next paining / rendering time (?)
  • The expected time at which each algorithm running in parallel may schedule a task
    • This is annoying one. It would mean that in order to correctly implement this feature in WebKit, we'd need to go find every & each algorithm that runs in parallel, and audit whether it can schedule a task, and if so decide whether I can estimate such the time at which it does so.
@yoavweiss yoavweiss referenced a pull request that will close this issue Feb 10, 2019
@rniwa

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Now that the HTML spec has explicit task sources when queuing tasks from algorithms that run in parallel, maybe can tie the concept of deadline to that. In particular, maybe we can say the deadline is reached when a new task is being queued to the user interaction task sources?

@rniwa

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

@yoavweiss

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

/cc @spanicker @shaseley - opinions on #71 (comment)?

@bzbarsky

This comment has been minimized.

Copy link

commented Jun 12, 2019

That wouldn't address the timer issue, right? It would address user input preempting an idle period, though and seems sensible in that regard. It wouldn't be able to affect the deadline calculation, though, unless I'm missing something.

@rniwa

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

@bzbarsky : Right, we have to handle the timer timeouts separately from the user inputs.

So my straw man proposal for when the deadline becomes 0 is as follows:

  1. The minimum time at which next timer or rAF callback fires.
  2. The time at which the user interaction task source gets a new task enqueued.
@bzbarsky

This comment has been minimized.

Copy link

commented Jun 12, 2019

So there are two meanings of "deadline", right?

  1. What value is passed to the idle callback?
  2. When do new idle callbacks stop getting run?

That proposal works for item (2), but I don't see how item (1) can depend on "The time at which the user interaction task source gets a new task enqueued", because that time is not known when the call happens, right?

@rniwa

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

@bzbarsky : Right. I think (1) will depend on timers & rAF for now. We can add more things to the list as we find them.

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Right, for (2) we already have a hook in the step 4 of the IdleDeadline timeremaining() interface to set it to zero if a "time-critical task pending". I agree we could use the new user interaction task source to make this phrasing more precise.

@rniwa

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

time-critical task pending

As I mentioned in #78, I don't think the phrase "time-critical task" would be any more precise than arbitrary time UA picks since there is no definition for being time critical.

@rmcilroy

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

That's why I suggested changing that phrasing to be based on the new new user interaction task source.

@yoavweiss yoavweiss self-assigned this Jun 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.