Skip to content

Minimal annot invalidation workaround #58112

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

Merged

Conversation

tecosaur
Copy link
Contributor

After discussion with Cody, this is an attempt at a more minimal version of #56194.

The intent is to work around the invalidation issue introduced by the split design with AnnotatedStrings, resolving the headache for 1.12.

Paired with JuliaLang/StyledStrings.jl#115

Many thanks to @topolarity for working out this approach, the discussion on balancing the short/long term fixes for this issue.


From my understanding of the problem, this should fix the write/print/show invalidations, but this needs to be checked.

@tecosaur tecosaur added stdlib Julia's standard library invalidations labels Apr 14, 2025
@topolarity
Copy link
Member

Just to clarify the approach here:

This does not resolve any actual type-piracy. Instead, it inserts an invalidation barrier around the existing StyledStrings type-piracy , similar to what we do in Logging. This should help with latency, but it does not resolve issues like JuliaLang/StyledStrings.jl#91

My personal preference would be one of the other pathways we've sketched that resolve type-piracy fully, but @tecosaur wanted an interim solution more quickly

@KristofferC
Copy link
Member

Some timings w.r.t #57997 (comment) could perhaps be done to check if this approach fixes the symptom of the piracy.

@KristofferC
Copy link
Member

KristofferC commented Apr 15, 2025

This seems to work ok to fix the #57997 latency:

julia> using Pkg


julia> Pkg.UPDATED_REGISTRY_THIS_SESSION[] = true
true

julia> ENV["JULIA_PKG_PRECOMPILE_AUTO"] = 0
0

julia> @time @eval Pkg.add("Example")
   Resolving package versions...
No packages added to or removed from `~/.julia/environments/v1.12/Project.toml`
No packages added to or removed from `~/.julia/environments/v1.12/Manifest.toml`
  0.207511 seconds (776.73 k allocations: 62.073 MiB, 8.57% gc time, 67.03% compilation time: <1% of which was recompilation)

Also better with e.g.

# branch
julia> @time @eval Base.Precompilation.precompilepkgs()
  0.049761 seconds (171.77 k allocations: 9.485 MiB, 96.97% compilation time: 86% of which was recompilation)

# 1.12-nightly
julia> @time @eval Base.Precompilation.precompilepkgs()
  0.235922 seconds (1.17 M allocations: 60.883 MiB, 17.97% gc time, 97.54% compilation time: 97% of which was recompilation)

@topolarity topolarity added the backport 1.12 Change should be backported to release-1.12 label Apr 15, 2025
@tecosaur tecosaur force-pushed the minimal-annot-invalidation-workaround branch 2 times, most recently from 2ae2d60 to 7f8a222 Compare April 16, 2025 13:38
@KristofferC KristofferC mentioned this pull request Apr 16, 2025
51 tasks
@tecosaur tecosaur force-pushed the minimal-annot-invalidation-workaround branch 3 times, most recently from 80f97e4 to f2e6a15 Compare April 21, 2025 10:24
@topolarity
Copy link
Member

CI is looking good. Does the StyledStrings PR need to merge before we merge this one?

@tecosaur
Copy link
Contributor Author

Nope, I can (will) fast-forward merge, putting the referenced commit on main without changing the hash — if CI hadn't spotted any issues and it looks good to you, I think we're good to merge this 🙂

@tecosaur tecosaur force-pushed the minimal-annot-invalidation-workaround branch from f2e6a15 to ba6f9dd Compare April 22, 2025 01:34
tecosaur and others added 2 commits April 22, 2025 21:51
Many headaches have been caused by the split implementation of
Annotated* display. This is an area that will require more work, however
for now the pain can be alleviated with an invalidation barrier.

Co-authored-by: Cody Tapscott <topolarity@tapscott.me>
@tecosaur tecosaur force-pushed the minimal-annot-invalidation-workaround branch from ba6f9dd to 9e04bd5 Compare April 22, 2025 13:51
Copy link
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for taking action on this issue @tecosaur, this should relieve invalidation pains significantly on 1.12

@topolarity topolarity merged commit 6d78a4a into JuliaLang:master Apr 22, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12 invalidations stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants