Skip to content

Clean up pthread startup code (NFC) #12308

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

Merged
merged 2 commits into from
Sep 23, 2020
Merged

Clean up pthread startup code (NFC) #12308

merged 2 commits into from
Sep 23, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 22, 2020

There were two places that pthread workers were initialized, one in a postSet,
and one at the end of postamble. This joins the two into a single initWorker
method, and moves code to there, which is simpler.

(Also add a missing initWorker for minimal runtime, so this is technically
not NFC for that runtime. Note that there may be more fixes needed there,
this just does the obvious thing for this PR to improve things.)

This will make fixing a race condition in a subsequent PR easier.

@kripken kripken requested review from juj and sbc100 September 22, 2020 23:47
@sbc100
Copy link
Collaborator

sbc100 commented Sep 23, 2020

What was wrong with doing the init work in postset?

@kripken
Copy link
Member Author

kripken commented Sep 23, 2020

Nothing wrong with a postset, this is just a cleanup. One place doing initialization is cleaner than two I think.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 23, 2020

Isn't it cleaner to do it in postset and avoid any code in postable/preamble completely?

(Full disclaimer I don't really get how postset works).

@kripken
Copy link
Member Author

kripken commented Sep 23, 2020

Oh, if we unify them then it can't be in a postset. A postset is emitted together with the JS library deps of a thing. That is, it ends up in some "random" location in the middle (as we emit the JS library things while we parse what is needed and recursively parse their deps and so forth - deterministic, but if you add another import from the JS library the entire order can change). The other mechanism has a clear fixed position, which is near the end of the entire file, and after necessary things (like JS library stuff) all exist. So if we unify them, it has to be in the other place, and not the postset.

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

Successfully merging this pull request may close these issues.

2 participants