w3c / longtasks

Long Task API
https://w3c.github.io/longtasks/
Other
246 stars 32 forks source link

Event loop timing reporting seems to ignore reentrancy #76

Closed annevk closed 1 year ago

annevk commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

npm1 commented 4 years ago

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 commented 4 years ago

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

npm1 commented 4 years ago

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 commented 4 years ago

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

npm1 commented 4 years ago

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 commented 4 years ago

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 https://github.com/w3c/longtasks/issues/83 for that.

annevk commented 4 years ago

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

mmocny commented 4 years ago

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"...]

yoavweiss commented 2 years ago

/cc @noamr

noamr commented 2 years ago

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 commented 1 year ago

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