-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Parallelize precompiles generation II #48049
Parallelize precompiles generation II #48049
Conversation
For me the gains are bigger before #47983
This PR
|
Half a minute, as advertised in the original PR 😉 |
Seems good to me. Does the switch to serial mode for debugging with stdout rather than devnull still work? |
Not sure, what you mean by |
I just grouped all the debug options together at the top of the file to make it clearer. Seems to work for me, if you disable pretty printing, disable parallel, and enable repl input viewing. edit: Had to move them to inside the module after stdout had been reinitialized |
d4f8c65
to
b39c4bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me, but might be good to have a @vtjnash review too
Out of curiosity I reordered Not sure if it's worth doing as it breaks the ordering (could be a followup PR), but at least it makes sense that doing the larger workloads first helps parallelization
|
julia/contrib/generate_precompile.jl Line 289 in cc5bc87
|
I'm kind of surprised, it saved you so much time. I believe, we can reorder repl script as we want because it will remain deterministic. |
ok! Pushed |
Windows failures are unrelated. @vtjnash does this look ok to you? |
I'll merge as this appears to work as designed. If @vtjnash has any issue with the buffering design that can be addressed in a followup |
EDIT: I can't reproduce this, so let's hope it was a fluke 🙂 I just had a build hang (wasn't using the changes from this PR yet):
As there was no CPU usage, but the main process was still happily spinning, I suspect this may be related to these parallel precompilation changes. The process also wasn't interruptible, i.e., CTRL-C is ignored (as you can see in the output). Are either of these issues known? |
We really need to get better at making ctrl-C work. I know that @vtjnash will tell me that it can't be made to work completely reliably, which, sucks, sure, but it can be made to work much better than it does and we should do that. |
@maleadt if you do hit it again, can you SIGINFO/SIGUSR1 profile it to see what it's doing. That's probably more reliable than getting a trace from an interrupt |
I've noticed a hang when building without stdlibs, i.e., |
LGTM. Copying streams (as done here) is one of the best actual use cases for |
This follows up #47983 and introduce asynchronous read for repl precompiles. In contrast to IanButterworth@5ee671a?w=1 from @IanButterworth , it uses
readavailable
andBufferStream
to handle end of lines correctly. It saves only a few seconds, but improves the fancy printing for impatient developers.