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

Don't monkey patch the event loop algorithm #57

Closed
annevk opened this issue Apr 18, 2017 · 12 comments
Closed

Don't monkey patch the event loop algorithm #57

annevk opened this issue Apr 18, 2017 · 12 comments
Milestone

Comments

@annevk
Copy link
Member

annevk commented Apr 18, 2017

Please integrate with HTML directly.

@plehegar
Copy link
Member

Are you suggesting that the spec should be moved into HTML directly or something else? If something else, how would you expect the integration to go since the idle periods are intended to be happening outside of event loops?

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

They cannot happen outside of the event loop. You can integrate via some kind of hook that you'd have to add to HTML if you don't want to integrate it wholesale as we did with animation frames.

@plehegar
Copy link
Member

RIC says "Whenever the user agent assesses that a given event loop is likely to remain idle for a non-trivial amount of time, [...], it SHOULD initiate a new idle period for the event loop. [...] When the user agent wishes to start an event loop's idle period [...]" So, if I read the spec properly, it's basically replacing temporarily the event loop with a different one. The spec definitively needs tightening in its various algorithms...

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

Yeah, I don't think we should do it that way. We should just have a very low-priority task source for idle tasks that we only run if the other higher priority sources are empty.

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

That would probably not even require all that much integration, since user agents are already allowed to prioritize task sources as they see fit.

@domenic
Copy link
Contributor

domenic commented Apr 19, 2017

Yes, running code outside the event loop breaks all kind of fundamental platform invariants; this definitely needs integration.

@domenic
Copy link
Contributor

domenic commented Apr 19, 2017

And, I also agree that this could probably be done without any changes to the HTML Standard itself, just by defining a new task queue and making some comments about how when user agents select a task queue at the top of the event loop algorithm, this one should be deprioritized. (I guess, unless the deadline is imminent/passed, in which case it should be highly prioritized.)

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

Making HTML maintain a list of task sources would be useful at some point though. Plus some advice on how to pick them. There's definitely some refactoring there that's worth doing at some point.

@plehegar plehegar added this to the Level 1 milestone May 16, 2017
@rmcilroy
Copy link
Contributor

I'm not entirely sure how to go about this change. As I see it, everything except for the algorithm in section 5.1 (Start an event loop's idle period) runs as standard tasks on the event loop. We already have a low priority task source for these tasks (see the "idle-task task source" in section 5.1).

However, just having a low-priority task source isn't enough - we need to be able to enable / disable that task source (which is what the algorithm in section 5.1 does). For example, even if there is no other higher priority tasks on the event loop, the user-agent shouldn't run rICs if it hasn't yet drawn the current frame (when animating/scrolling/etc.). We also want to prevent a new idle period from starting before a previous one would have finished (see 2.1 in section 5.1)

Would it be enough to make the algorithm in section 5.1 into a task which is run on either the event loop (either from the normal task source or the idle-task task source, I'm not sure which would make most sense right now)?

@annevk
Copy link
Member Author

annevk commented Jul 27, 2017

I don't think you can turn it into a task since that might run too soon per your criteria as well. With those requirements you probably do need changes to the HTML Standard.

rmcilroy added a commit to rmcilroy/requestidlecallback that referenced this issue Jul 28, 2017
rmcilroy added a commit to rmcilroy/requestidlecallback that referenced this issue Jul 28, 2017
rmcilroy added a commit to rmcilroy/requestidlecallback that referenced this issue Jul 28, 2017
rmcilroy added a commit to rmcilroy/requestidlecallback that referenced this issue Jul 28, 2017
@rmcilroy
Copy link
Contributor

I had a bash at changing the "start idle period" part of the algorithm to be a task in #60, I think it should still provide similar semantics to the current spec. Please take a look and let me know what you think.

@igrigorik
Copy link
Member

Closing, resolved via #60.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants