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

Clean up the event loop and agent correspondence #5396

Merged
merged 2 commits into from
Mar 26, 2020

Conversation

domenic
Copy link
Member

@domenic domenic commented Mar 25, 2020

This closes #5352, by making event loops and agents 1:1. It also adds an
explicit explanation that event loops/agents and implementation threads
are not necessarily 1:1, apart from the restrictions imposed by
JavaScript's forward progress guarantee.

This also closes #4213, as worklet event loops work fine in this
architecture.

This also closes #4674, as browsing contexts changing event loops is
not a problem in this architecture, since they change agents at the same
time.

Finally, this guards one step in the event loop processing model that
only made sense for window event loops.


I'm not sure this truly closes #4674, so please check my reasoning. In general I have a hard time understanding the paragraph there; if you think it still applies, then let's discuss. Maybe it needs needs to move to a different location, e.g. the agents section?

Let's have that discussion in #4674, I guess, to keep it consolidated.


/infrastructure.html ( diff )
/webappapis.html ( diff )

@domenic domenic added topic: agent The interaction with JavaScript's agent and agent cluster concepts topic: event loop labels Mar 25, 2020
@domenic
Copy link
Member Author

domenic commented Mar 25, 2020

Finally, this guards one step in the event loop processing model that
only made sense for window event loops.

This step was more broken than I realized. #5397 has a better fix than the one in this PR, and should ideally land first.

arising from <span data-x="navigate">navigating</span> between <span data-x="similar-origin window
agent">similar-origin window agents</span>. E.g., when a <span>browsing context</span> <span
data-x="navigate">navigates</span> from <code data-x="">https://example.com/</code> to <code
data-x="">https://shop.example/</code>.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I had in mind with this note is mostly that we don't describe how communication between agents works that well, in some cases in observable ways (e.g., otherWindow.postMessage() followed by otherWindow.focus() can result in focus() happening second). But I'm okay removing it as we have issues covering these problems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll mention that in the commit message, at least.

This closes #5352, by making event loops and agents 1:1. It also adds an
explicit explanation that event loops/agents and implementation threads
are not necessarily 1:1, apart from the restrictions imposed by
JavaScript's forward progress guarantee.

This also closes #4213, as worklet event loops work fine in this
architecture.

This also closes #4674, as browsing contexts changing event loops is
not a problem in this architecture, since they change agents at the same
time. Other problems remain around communication between agents (see
e.g. #3691), and some of that communication is event-loop mediated, but
the specific note discussed there is no longer relevant.
source Outdated
Comment on lines 88936 to 88937
<p>However, because the various worker <span data-x="agent">agents</span> are allocated with
[[CanBlock]] set to true, the JavaScript specification does place requirements on them regarding
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<p>However, because the various worker <span data-x="agent">agents</span> are allocated with
[[CanBlock]] set to true, the JavaScript specification does place requirements on them regarding
<p>However, for the various worker <span data-x="agent">agents</span> that are allocated with
[[CanBlock]] set to true, the JavaScript specification does place requirements on them regarding

But maybe this needs to be something else? The main problem here is that a service worker agent does not have [[CanBlock]] set to true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Yeah, I think the JS spec does not prohibit sharing a thread between the service worker and the main thread.

Does the HTML spec want to do so? I mean, I could imagine a workable implementation that cooperatively schedules the two threads... we've been careful to avoid blocking APIs in service workers...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit related: #4339

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so it's possible that being in separate agent clusters would prohibit such sharing, although it's not obvious to me from the JS spec. If that's the boundary then this note should also mention that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked a bit with @syg who confirmed that the JS spec does not require parallelism between separate agent clusters, and also pointed out that such a requirement would be unobservable. Without sync cross-agent cluster communication (like we have sync cross-[[CanBlock]]-agent communication via atomics), there's no way to tell whether you're dealing with parallelism or cooperative scheduling.

So, I think this sentence we're commenting on is basically correct. It restricts itself to talking about [[CanBlock]], which is the only real observable requirement.

@domenic domenic merged commit cd59059 into master Mar 26, 2020
@domenic domenic deleted the event-loops-and-agents branch March 26, 2020 17:18
domenic added a commit that referenced this pull request Mar 31, 2020
Closes #5210.

Closes #4339 by removing the explicit phrasing of a parallel execution
environment, instead relying on the agent infrastructure.

Closes #4988 by reintroducing a definition for "similar-origin window
agent", separate from its creation algorithm. Like the definition of all
agent types, it is now informal, just stating what globals are contained
in the agent.

Relocates the site-related definitions to the "Origin" section of the
spec, adjacent to the "schemelessly same site" and "same site"
definitions.

Makes event loop creation explicit during agent creation, now that they
are 1:1 per #5396 and #5416.

Makes "responsible event loop" a convenience accessor, instead of one of
the core algorithms of environment settings objects. This means that
individual environment settings object definitions no longer need to
specify how to retrieve the responsible event loop.

Exposes some of the higher-level concepts here to the dev edition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: agent The interaction with JavaScript's agent and agent cluster concepts topic: event loop
2 participants