Skip to content

Conversation

timholy
Copy link
Member

@timholy timholy commented Mar 19, 2025

With the "classic" inference timing disabled, PrecompileTools and SnoopCompile are non-functional on 1.12 & master. #57074 added timing data to the CodeInstances themselves, which should help restore SnoopCompile. However, without the tree structure provided by the old inference timing system, we still need a way to tag MethodInstances that get inferred inside PrecompileTools.@compile_workload blocks.

This adds a simple mechanism modeled after @trace_compile that switches to tagging MethodInstances in jl_push_newly_inferred. JuliaLang/PrecompileTools.jl#47 contains (or will contain) the corresponding changes to PrecompileTools.jl.

With the "classic" inference timing disabled, PrecompileTools and
SnoopCompile are non-functional on 1.12 & master. #57074 added timing
data to the CodeInstances themselves, which should help restore
SnoopCompile. However, without the tree structure provided by the old
inference timing system, we still need a way to tag MethodInstances that
get inferred inside `PrecompileTools.@compile_workload` blocks.

This adds a simple mechanism modeled after `@trace_compile` that
switching to tagging MethodInstances in `jl_push_newly_inferred`.
JuliaLang/PrecompileTools.jl#47 contains the
corresponding changes to PrecompileTools.jl.
@vtjnash vtjnash added the backport 1.12 Change should be backported to release-1.12 label Mar 19, 2025
@IanButterworth
Copy link
Member

Will Pkg's internalized @compile_workload need updating?

https://github.com/JuliaLang/Pkg.jl/blob/master/src/precompile.jl#L217-L244

@timholy
Copy link
Member Author

timholy commented Mar 19, 2025

Yes, I'll push a very simple test in the next commit. (PrecompileTools is a test of sorts, but good to have one here too.)

@timholy timholy added the merge me PR is reviewed. Merge when all tests are passing label Mar 19, 2025
@timholy
Copy link
Member Author

timholy commented Mar 19, 2025

Corresponding changes in JuliaLang/PrecompileTools.jl#47 are now up too

@IanButterworth
Copy link
Member

I just noticed that #54899 deleted the corresponding internalized @compile_workload code from REPL. So that will need reinstating also.

Shall we make a barebones version for Base so that we're not duplicating code between Pkg and REPL and any other stdlibs that want to benefit?

@vtjnash
Copy link
Member

vtjnash commented Mar 19, 2025

I deleted it there because I noticed it seemed to be making the quality of the REPL script worse not better

@JeffBezanson JeffBezanson merged commit c89b1ff into master Mar 20, 2025
7 of 8 checks passed
@JeffBezanson JeffBezanson deleted the teh/pctagging branch March 20, 2025 21:01
KristofferC pushed a commit that referenced this pull request Mar 20, 2025
With the "classic" inference timing disabled, PrecompileTools and
SnoopCompile are non-functional on 1.12 & master. #57074 added timing
data to the CodeInstances themselves, which should help restore
SnoopCompile. However, without the tree structure provided by the old
inference timing system, we still need a way to tag MethodInstances that
get inferred inside `PrecompileTools.@compile_workload` blocks.

This adds a simple mechanism modeled after `@trace_compile` that
switches to tagging MethodInstances in `jl_push_newly_inferred`.
JuliaLang/PrecompileTools.jl#47 contains (or
will contain) the corresponding changes to PrecompileTools.jl.

(cherry picked from commit c89b1ff)
@timholy timholy removed the merge me PR is reviewed. Merge when all tests are passing label Mar 21, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Mar 24, 2025
// Mutex for newly_inferred
jl_mutex_t newly_inferred_mutex;
extern jl_mutex_t world_counter_lock;
static _Atomic(uint8_t) jl_tag_newly_inferred_enabled = 0;
Copy link
Member

Choose a reason for hiding this comment

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

This could just be a UInt64 since we are using a whole cache line anyways

vchuravy added a commit that referenced this pull request Oct 16, 2025
REPL precompile scripts runs a workload and might thus encounter code in
other standard libraries that needs to be precompiled. Before #54899 we
had a bespoke variant of PrecompileTools.jl. PrecompileTools was fixed
with #57828 so we can now re-instate the support in REPL.

Noticed by @tecosaur, while looking at #51811
KristofferC pushed a commit that referenced this pull request Oct 16, 2025
REPL precompile scripts runs a workload and might thus encounter code in
other standard libraries that needs to be precompiled. Before #54899 we
had a bespoke variant of PrecompileTools.jl. PrecompileTools was fixed
with #57828 so we can now re-instate the support in REPL.

Noticed by @tecosaur, while looking at #51811

(cherry picked from commit ecfec85)
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.

6 participants