-
-
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
WIP: generate precompile as part of build process #28118
Conversation
We have some logic somewhere (though I can't recall where offhand) that checks whether the current Git SHA matches that of a tagged release. We could use that to conditionally enable this. |
e4d2c68
to
43329d3
Compare
Wouldn't the tagging happen after the release is built? |
We'd definitely want to do this before tagging a release to verify it's working, no? Maybe do it by default but offer an option to be set in Make.user to disable it? |
43329d3
to
3a0396d
Compare
Small update. I fixed the problem that not all things got recorded so this is now just as good as the manual version in #28075. Seems to have been some improvements on master as well so with this we now have
So almost 3x faster startup vs beta. REPL is also very fast (first beta then PR: https://asciinema.org/a/rjvVIEm8gNd8KskeitmJCNPVU). Improvement is also visible on CI:
vs
This is what I have done now. Double building the sysimg on CI is arguably a bit unfortunate though? Does someone understand why AV can't find the files in
Trying the build bots to see how they fare: |
3a0396d
to
e8c9925
Compare
Yes. But we also want this tested. |
I'll write down the process this uses to get feedback.
|
92e187e
to
7a5f792
Compare
So it seems like the main question is when to do this during CI builds. On the one hand doing it all the time seems wasteful and somewhat slow, on the other hand not doing it all the time risks this process bitrotting. Is that the consideration or is there something else? |
Could this process somehow keep track of the hash of the function, and only re-precompile a function if it changes? |
That seems roughly equivalent to checking in the generated precompiles and doing an extra sysimg compile round when it changes. I wonder if there’s a way to tell from the diff if the old precompiles will straight up fail or if they will merely be suboptimal? |
FWIW, this is precisely equivalent to building it once, and saving the result after running whatever steps created the precompile.jl file. Or, you can equivalently add a second precompile file into the process during the |
Okay I can see how I can probably get the REPL interaction precompile statements without generating the sysimg twice but the ones that come from the actual startup, don't I need an already built julia executable for that? |
You would test this by creating a
It seems to me that what we really want is almost like a git prehook check that figures out the "optimal" precompile statements dynamically and auto-updates the correct list of precompile statements automatically. If bad precompile statements can break the build, I would argue we should do this generation as a part of every PR at least. If it can't break the build but can only make thing slower, seems like a good candidate for a weekly bot autorun or something. |
This is pretty much what this PR is doing, except it doesn't commit the precompile statements since they are so ephemeral (at least the ones for anonymous functions).
They can (symbols for anonymous functions disappearing), but we could just I don't see much reason checking these into version control though. Something similar to what we have here where they are an output from the build process itself seems better. |
7a5f792
to
195dd88
Compare
I don't think so. Or rather, no already built |
9488fda
to
9851be4
Compare
Ok, reworked this so it doesn't require an extra sysimg build and the precompile statements are instead generated when the Comments on the implementation appreciated. The reason why I am launching extra processes in
Note that this doesn't actually make startup time much faster (than the old precompile file) because most of the things that took time and was anonymous functions got moved to be lazily loaded. |
9851be4
to
fd07681
Compare
aeefa6a
to
38c8a03
Compare
This comment has been minimized.
This comment has been minimized.
Worked around #28308 by not using |
Any reason not to merge this right away? |
Since this touches the build system which feels more fragile for some reason, I think it should be looked over a bit first by those familiar with it. |
But this has been open for more than a week with few changes and should be very easy to revert since it doesn't touch any files that are really modified so let's try it out on the build bots and if they explode we can just revert. I'm quite excited to try the nightly with this to see if it works well... |
Just tried this. Best thing since sliced bread. The system image isn't even any bigger! Awesome work. |
Now I'm getting greedy:
:( |
I've seen a few left overs but they seem to be in the ms time range? They are also not anonymous so could just be explicitly added. |
The output during the build is a bit confusing. What is this telling me?
|
You mean the stuff in between the bars. It's just some output to stdout from the REPL replayer. Could probably be made more quiet. But it needs TTY for stdout which is why I am not redirecting it to a file or Pipe. |
Hm, we also still seem to get the |
Why does generate_precompile.jl explicitly call |
Because it needs libuv events to be setup to be able to use things like pipes. |
This reverts commit b43e7ad.
This reverts commit b43e7ad.
This reverts commit bb16216.
This reverts commit bb16216.
This is a WIP towards fixing #26503. I got inspired on working on it when doing this manually gave such big benefit (#28075).
So what this does is that a part of building the release build, julia is started with a (new) flag that emits precompile statements to STDOUT, a fake REPL (copy pasted from the REPL test code) is spun up, and commands from a file are fed into that REPL. The recorded precompile statements are then stored in a file that the
precompile.jl
file includes. The sysimg is then rebuilt with those precompile statements.Things TODO:
The test REPL uses aSearch and replaceFakeTerminal
while the real REPL uses aTTY
. This means that a lot of the precompile statements that depend on the Terminal are not applicable.FakeTerminals.FakeTerminal
forREPL.Terminals.TTYTerminal
works well enough...This now builds the sysimg twice onmake
. Annoying for development. Not sure how it is best to only do this when you want to make a real release.I know almost nothing about Makefile so everything here is just from looking at the surrounding code. Help appreciated.This doesn't manage to record everything that the manual version does. We can see what it misses:Update: fixed.But after running this, startup of julia goes from 0.4s to 0.2s on my computer and first command in the REPL is instant etc. We could also record e.g. the status output for Pkg to make that quick if we want to.