Conversation
xref JuliaLang/julia#57074 The old `Compiler.Timings` infrastructure is disabled, so here we leverage the same `newly_compiled` infrastructure used during precompilation.
|
The failing test is a failure to precompile |
|
Curiouser and curiouser. This diff: diff --git a/src/PrecompileTools.jl b/src/PrecompileTools.jl
index c76a98b..9515aef 100644
--- a/src/PrecompileTools.jl
+++ b/src/PrecompileTools.jl
@@ -7,11 +7,17 @@ export @setup_workload, @compile_workload, @recompile_invalidations
const verbose = Ref(false) # if true, prints all the precompiles
function precompile_mi(mi::Core.MethodInstance)
- precompile(mi.specTypes) # TODO: Julia should allow one to pass `mi` directly (would handle `invoke` properly)
+ precompile(mi)
verbose[] && println(mi)
return
end
+function precompile_ci(ci::Core.CodeInstance)
+ precompile(ci.def)
+ verbose[] && (println(ci.def); @show ci.precompile)
+ return
+end
+
include("workloads.jl")
include("invalidations.jl")
diff --git a/src/workloads.jl b/src/workloads.jl
index 0480b1b..b40b067 100644
--- a/src/workloads.jl
+++ b/src/workloads.jl
@@ -14,8 +14,7 @@ end
function precompile_newly_inferred(cis)
while !isempty(cis)
- ci = pop!(cis)
- precompile_mi(ci.def)
+ precompile_ci(pop!(cis))
end
return cis
end
diff --git a/test/PC_A/src/PC_A.jl b/test/PC_A/src/PC_A.jl
index e524ab6..dde4396 100644
--- a/test/PC_A/src/PC_A.jl
+++ b/test/PC_A/src/PC_A.jl
@@ -1,6 +1,8 @@
module PC_A
-using PrecompileTools: @setup_workload, @compile_workload
+using PrecompileTools: PrecompileTools, @setup_workload, @compile_workload
+
+PrecompileTools.verbose[] = true
struct MyType
x::Intyields the following output: So it doesn't seem to be cached despite having been tagged |
|
timholy/PkgCacheInspector.jl#33 shows that the failure is in the caching step (that MethodInstance is not part of the cache). |
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #47 +/- ##
==========================================
- Coverage 82.97% 81.81% -1.17%
==========================================
Files 3 3
Lines 94 77 -17
==========================================
- Hits 78 63 -15
+ Misses 16 14 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Am I right in thinking we need oldcache = steal_newly_inferred()
try
<workload>
finally
restore_newly_inferred(oldcache)
precompile_newly_inferred()
endwhere |
|
Note that this is passing, however, so I think your fix worked. Thanks! |
There are some thread-safety problems with that, so I opted not to suggest that. Each block sets up its own newly_inferred state, so that is not particularly an issue, and we likely don't want to include the code in between compile_workload blocks either. It however does mean they wouldn't nest successfully (the first nested block will terminate tracking for all of the parents also), but I don't know if there's really anything to be done about that, unless you wanted to add a ScopedValue to neutralize the entire |
|
I'm less worried about nesting than @compile_workload begin
workload1
end
const default = compute_defaults()With this current design, we may lose external CodeInstances that get compiled during the call |
|
Here's a thought: what if we define (in Base) a pair of "indicator" and similarly for |
|
If we're changing base, it might be easier to piggy back on the |
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.
|
Convenient link: JuliaLang/julia#57828 |
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 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)
|
This passes locally on latest 1.12 so I think it can be merged. |
|
Hah, I was looking at this too. Is the fact that it failed with the old Manifest a concern? Won't this be rampant across the ecosystem? |
|
Even though I had gotten all 6 checks passing, it still wouldn't let me merge. I had to un-check this ruleset:
Seem OK? Not sure why it was checked when no one was listed among those with bypass permissions. Or is it better to recheck it and then start listing people? |
|
I never found these restrict things in repos to have any value so ok to me to keep it unchecked. |
When you update the julia version to run the docs with you should regenerate the manifest. |
3 participants
xref JuliaLang/julia#57074
The old
Compiler.Timingsinfrastructure is disabled, so here weleverage the same
newly_compiledinfrastructure used duringprecompilation.
CC @vtjnash. There's one failing test that needs some investigation.