Skip to content
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

Ensure compilation happens after logging starts #53

Merged
merged 2 commits into from
Apr 3, 2025
Merged

Conversation

timholy
Copy link
Member

@timholy timholy commented Apr 2, 2025

If compilation of the workload happens before the logging system is
initialized, many CodeInstances may be missed. This exploits
Core.@latestworld to prevent inference from proceeding into the
workload. It removes the @force_compile, which introduces
some risk that even the workload will be interpreted.

Also wraps the @setup_workload block with a let.

Fixes #17
Fixes #26
Fixes #37
Closes #40
Closes JuliaLang/julia#57957

CC @vtjnash

If compilation of the workload happens before the logging system is
initialized, many CodeInstances may be missed. This exploits
`Core.@invokelatest` to prevent inference from proceeding into the
workload. It removes the `@force_compile`, which introduces
some risk that even the workload will be interpreted.

Also wraps the `@setup_workload` block with a `let`.

Fixes #17
Fixes #26
Fixes #37
@timholy
Copy link
Member Author

timholy commented Apr 2, 2025

Weird. Those tests pass locally for me. I'm guessing it's a coverage thing.

Copy link

codecov bot commented Apr 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.81%. Comparing base (1e738ac) to head (e5ec2f6).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #53   +/-   ##
=======================================
  Coverage   81.81%   81.81%           
=======================================
  Files           3        3           
  Lines          77       77           
=======================================
  Hits           63       63           
  Misses         14       14           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KristofferC
Copy link
Member

Is there something that can be backported to a pre-1.12+ julia PrecompileTools version so that the compile fixes are also available there?

@timholy
Copy link
Member Author

timholy commented Apr 2, 2025

No, Core.@invokelatest was only added in Julia 1.12. EDIT: although it's possible that hiding the ccall behind a @noinline could help a bit.

@KristofferC
Copy link
Member

Core.@latestworld I guess we mean.

although it's possible that hiding the ccall behind a @noinline could help a bit.

Or maybe @invokelatest tag_newly_inferred_enable()... I'm just throwing out stuff

@timholy
Copy link
Member Author

timholy commented Apr 2, 2025

Yes I meant Core.@latestworld, sorry. I (strongly) doubt the @invokelatest would help.

@timholy
Copy link
Member Author

timholy commented Apr 2, 2025

I could add a branch for pre-Julia 1.12 to make it easier to continue to support older releases. Since this repo is JuliaLang, any preferred naming convention? If not I may use before-julia-1.12.

@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2025

Before v1.12, you could conditionally define something like macro latestworld() esc(Expr(:toplevel, nothing)) end to attempt to influence the heuristics (or perhaps Expr(:copyast, nothing) if forced toplevel is too strong)

@timholy timholy merged commit 980071c into main Apr 3, 2025
6 checks passed
@timholy timholy deleted the teh/fix_comporder branch April 3, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants