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

Event loop timing reporting seems to ignore reentrancy #76

Closed
annevk opened this issue Nov 22, 2019 · 13 comments
Closed

Event loop timing reporting seems to ignore reentrancy #76

annevk opened this issue Nov 22, 2019 · 13 comments
Assignees

Comments

@annevk
Copy link
Member

annevk commented Nov 22, 2019

The event loop is currently still reentrant. It's not entirely clear to me the processing model takes that into account. That is, you might get the wrong results for certain tasks that perform expensive operations.

@npm1
Copy link
Contributor

npm1 commented Nov 22, 2019

So a task can be paused for an arbitrary amount of time and then that same task would be resumed? It's true that this would probably introduce a longtask. Chrome's tasks do not have this behavior, does Firefox do this kind of thing in practice?

@annevk
Copy link
Member Author

annevk commented Nov 24, 2019

Firefox has a nested event loop for synchronous XMLHttpRequest, alert(), and similar features that can be observed to some extent.

@npm1
Copy link
Contributor

npm1 commented Nov 27, 2019

But if I'm understanding this correctly, these nested event loops are limited in what they can do, and in particular they cannot process user input right? I'm not sure where the HTML spec covers reentrancy of the event loop but for the purpose of this spec a longtask is a chunk of work which the browser executes without looking at other tasks (which means it cannot process user input while this chunk of work is executing).

@annevk
Copy link
Member Author

annevk commented Dec 3, 2019

It's just the normal event loop, so anything that's queued goes.

@npm1
Copy link
Contributor

npm1 commented Dec 3, 2019

So if the task is stopped and the browser is now able to handle new high priority tasks (such as input), that would constitute the end of the current task. If it stops because it's awaiting on something but is not allowed to run other tasks while awaiting then it's not the end of the current task. Which kind of reentrancy is it and where is it specified?

@annevk
Copy link
Member Author

annevk commented Dec 4, 2019

It's stopped, but then later resumed. It's part of HTML, see "spin the event loop" for instance.

@npm1
Copy link
Contributor

npm1 commented Dec 4, 2019

So the 'spin the event loop' could be expanded to change the timing variables when stopped and when resumed so the task is broken into two separately timed parts. But I think it would be clearer for the 'spin' to define the work after resuming as a new task.

@npm1
Copy link
Contributor

npm1 commented Apr 9, 2020

Discussed at the call, people seemed to agree that an sync xhr or an alert() blocking other work should be punished, so it's fine that it would produce a longtask. In the future we do want to ensure that there is sufficient attribution for tasks blocked on users, so filed #83 for that.

@annevk
Copy link
Member Author

annevk commented Apr 14, 2020

Is it actually punishing it though? Did someone write a test?

@mmocny
Copy link
Contributor

mmocny commented Apr 20, 2020

If I understand @annevk, sync XHR/alert() in FF will unblock the event loop, allowing other tasks to move forward, even when the current task is blocked until completion.

Sounds similar to async-await, except automatically retrofitted into these older features (is that right?), yet the task duration is meant to span across this automatically applied yield.

[I also assume that alert() will still "prevent the user from accessing the rest of the program's interface until the dialog box is closed", so "can browser process user input" question is probably "yes, but there won't be any new input"...]

@npm1 npm1 reopened this Apr 20, 2020
@yoavweiss
Copy link
Contributor

/cc @noamr

@noamr noamr self-assigned this Apr 9, 2022
@noamr
Copy link
Contributor

noamr commented Apr 10, 2022

I want to separate this to two cases:

  1. pause, which is used by synchronous XHR and alert
  2. spin the event loop, which is used in "logical" cases such as waiting for resources that block the load event. Roughly equivalent to async/await.

I believe the former should affect the long task duration, and the latter should not.

Researching this a bit, the spec already does this.

In spin the event loop, the task is actually stopped, and a new task is queued with the remaining steps, which would create a new task time etc.

So I believe this is actually a no-op. I'll make sure it's properly covered by tests.

noamr added a commit to web-platform-tests/wpt that referenced this issue Apr 10, 2022
- sync XHR should contribute to longtask timing
- A long split task, such as waiting for window load event, is not a longtask

See w3c/longtasks#76
@noamr
Copy link
Contributor

noamr commented Feb 5, 2023

Closing this since it was covered by tests and the spec is clear about this.

@noamr noamr closed this as completed Feb 5, 2023
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

6 participants