-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Minimal annot invalidation workaround #58112
Conversation
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 |
Some timings w.r.t #57997 (comment) could perhaps be done to check if this approach fixes the symptom of the piracy. |
This seems to work ok to fix the #57997 latency:
Also better with e.g.
|
2ae2d60
to
7f8a222
Compare
80f97e4
to
f2e6a15
Compare
CI is looking good. Does the StyledStrings PR need to merge before we merge this one? |
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 🙂 |
f2e6a15
to
ba6f9dd
Compare
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>
ba6f9dd
to
9e04bd5
Compare
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.
LGTM.
Thanks for taking action on this issue @tecosaur, this should relieve invalidation pains significantly on 1.12
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.