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

Do not run FinishDynamicImport steps in parallel #4182

Merged
merged 1 commit into from
Nov 20, 2018

Conversation

domenic
Copy link
Member

@domenic domenic commented Nov 19, 2018

These steps do a variety of things that only make sense back on the main thread.
The intention here was that fetching not block the main thread. However, that is
already the case even without going in parallel, given the "async algorithm"
framework of the script-fetching algorithms. (That framework is admittedly a bit
unclear; see whatwg/infra#181.)

This is on top of #4181; please only review the second commit. If it turns out #4181 is going to take a while to land (e.g. due to tests) then I can rearrange the commits.

@annevk
Copy link
Member

annevk commented Nov 20, 2018

Second commit looks good, but I can't approve it in isolation.

@domenic
Copy link
Member Author

domenic commented Nov 20, 2018

You can approve the PR on the assumption that I'll do the right thing when it comes to merging...

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Fair.

@domenic
Copy link
Member Author

domenic commented Nov 20, 2018

I'm going to reverse the order as per #3295 (comment) and testing discussions #4181 is going to get a bit more complicated. So, land only the second commit of this one first.

These steps do a variety of things that only make sense back on the main thread.
The intention here was that fetching not block the main thread. However, that is
already the case even without going in parallel, given the "async algorithm"
framework of the script-fetching algorithms. (That framework is admittedly a bit
unclear; see whatwg/infra#181.)
@domenic domenic force-pushed the finish-dynamic-import branch from 8c99a14 to adf732f Compare November 20, 2018 19:46
@domenic domenic merged commit 4ee44da into master Nov 20, 2018
@domenic domenic deleted the finish-dynamic-import branch November 20, 2018 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants