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

Fix bad precompile statements via an output lock #44252

Merged

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Feb 19, 2022

It seems like multithreading might cause statements to be outputted at the same time, so a file lock is needed?

Fixes the latest reports in #28808 but shouldn't close that issue as the core issue there appears to be a different bug.

@IanButterworth IanButterworth added the building Build system, or building Julia or its dependencies label Feb 19, 2022
@IanButterworth IanButterworth changed the title Lock around outputting precompile statements. Fixes bad precompile statements during build Fix bad precompile statements via an output lock Feb 19, 2022
Copy link
Member Author

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

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

Probably worth only locking if threading is possible

src/gf.c Outdated Show resolved Hide resolved
src/gf.c Outdated Show resolved Hide resolved
@IanButterworth
Copy link
Member Author

@KristofferC this is fixed in base build by only using one thread during precompile statement generation, but I think this is still needed for PackageCompiler as the user may be trying to sysimage multithreaded code that can't be run single threaded. Does that sound right?

@KristofferC
Copy link
Member

That seems like it could indeed happen. Fixing it here seems like a better way than just doing it for the Base precompile script.

@IanButterworth
Copy link
Member Author

Ok I'll merge. This seems pretty safe and the nthreads checks mean there should be no slowdown on julia build.

@IanButterworth IanButterworth merged commit c5bc69c into JuliaLang:master Feb 21, 2022
@IanButterworth IanButterworth deleted the ib/fix_precomp_statements branch February 21, 2022 15:37
KristofferC pushed a commit that referenced this pull request Feb 23, 2022
@IanButterworth IanButterworth added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Feb 23, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Feb 24, 2022
staticfloat pushed a commit to JuliaCI/julia-buildkite-testing that referenced this pull request Mar 2, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@KristofferC KristofferC mentioned this pull request Mar 15, 2022
50 tasks
@KristofferC
Copy link
Member

This needs a manual backport against backports-release-1.6.

@KristofferC KristofferC mentioned this pull request Apr 19, 2022
40 tasks
@KristofferC KristofferC mentioned this pull request May 16, 2022
45 tasks
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants