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 that annotations are well-ordered in optimisation #53800

Closed

Conversation

tecosaur
Copy link
Contributor

@tecosaur tecosaur commented Mar 21, 2024

This is a follow-on from #53794 (which needs to be merged before this PR) that largely seems like a "might as well" type thing to me.

See the commit message for more details.

A cosmetic change this has me considering is whether it would be nice to rename this function to annotatedstring_normalize!. Thoughts?

The function annotatedstring_optimize! assumes, but does not ensure that
annotations are well ordered. For a small (~2%) performance cost, we can
ensure that annotations are indeed well-ordered, which seems worthwhile.
@tecosaur
Copy link
Contributor Author

tecosaur commented Apr 2, 2024

Let me know if I'm being too ambitious, but I'm going to speculatively tag this with backport-1.11 since it (IMO) improves the state of this feature in the 1.11 release.

@tecosaur tecosaur added backport 1.11 Change should be backported to release-1.11 status:awaiting review PR is complete and seems ready to merge. Has tests and news/compat if needed. CI failures unrelated. labels Apr 2, 2024
@KristofferC KristofferC mentioned this pull request Apr 9, 2024
41 tasks
@KristofferC KristofferC mentioned this pull request Apr 17, 2024
59 tasks
@tecosaur tecosaur removed status:awaiting review PR is complete and seems ready to merge. Has tests and news/compat if needed. CI failures unrelated. backport 1.11 Change should be backported to release-1.11 labels Apr 27, 2024
@tecosaur
Copy link
Contributor Author

After a long chat with @LilithHafner, we're going to be taking a different approach to this. A superseding PR should be up shortly.

@tecosaur tecosaur closed this Apr 27, 2024
@tecosaur tecosaur deleted the order-annotations-when-optimising branch April 27, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:strings "Strings!"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant