-
-
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
Fix some invalidations during Julia's bootstrap #36427
Conversation
Related: #36411 (that was not tested along with this, and the combination will get rid of a few more besides being more correct) |
@@ -266,6 +266,9 @@ function active_project(search_load_path::Bool=true) | |||
project == "@" && continue | |||
project = load_path_expand(project) | |||
project === nothing && continue | |||
# while this seems well-inferred, nevertheless without the type annotation below | |||
# there are backedges here from abspath(::AbstractString, ::String) |
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.
This seems very strange --- AFAICT we already know that project
is a String
here, so I don't know why this would be. We should investigate.
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.
Got it --- it's due to recursion via load_path_expand
. The types get narrowed later, but the edges added before the recursion resolved are still there.
base/shell.jl
Outdated
@@ -33,23 +33,25 @@ function shell_parse(str::AbstractString, interpolate::Bool=true; | |||
st = Iterators.Stateful(pairs(s)) | |||
|
|||
function update_arg(x) | |||
if !isa(x,AbstractString) || !isempty(x) | |||
push!(arg, x) | |||
let arg = arg |
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.
I wouldn't expect this to do anything --- arg
has a declared type, plus this function already only reads the variable.
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.
And it doesn't help, it's still boxed. I meant to back that part out, will do it now. s
and i
are now unboxed.
aeed06f
to
0b55425
Compare
Also add nospecialize to with_output_color
This cuts the total number of invalidations that occur during Julia's build process approximately in half. It may knock about 2% off Julia's build time, though that's about in the noise so I am not sure. Here was one run on
master
:and here were two runs on this(*caveat: also using #35877) branch:
Interestingly, both were about a second longer on generating the precompile statements, reducing or eliminating the total advantage. I don't know whether that's systematic. (With fewer to build, you might expect it to be faster, maybe?)
The remaining invalidations fit a couple of patterns:
reducedim.jl
, which specialize methods already in active use fromreduce.jl
promote_rule
invalidations due to specializing the fallbacks. None of these have large cascades of consequences, however.MethodInstances
(~80? I don't really remember), from code that might call a function before it has any methods defined (result: they mt-invalidate themselves):with_output_color
gets used by theshow
method incmd.jl
which gets used by Base.FileRedirect via the logging frameworkfill!
isn't defined until well after its first potential use byshow(io::IO, x::Type)
, via thevcat
callWhile trying to understand the
with_output_color
phenomenon I added a@nospecialize
, this is a bonus but it seems reasonable.I'm happy to back out some of these if folks prefer, just let me know. I was basically going down the list, but none of these are game-changing so we can afford to drop some.