-
Notifications
You must be signed in to change notification settings - Fork 110
BE-262: HashQL: Do not intern empty slices #8223
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
Conversation
PR SummaryIntroduces canonical empty interned slices and simplifies ID generation and map initialization.
Written by Cursor Bugbot for commit 752a68c. This will update automatically on new commits. Configure here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8223 +/- ##
==========================================
+ Coverage 59.24% 59.36% +0.12%
==========================================
Files 1191 1195 +4
Lines 113363 113729 +366
Branches 4981 4985 +4
==========================================
+ Hits 67159 67520 +361
- Misses 45428 45433 +5
Partials 776 776
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 13.31%
Performance Changes
Comparing Footnotes
|
🤖 Augment PR SummarySummary: Refactors HashQL interning collections to allocate their backing hash tables in the arena Changes:
Technical Notes: This keeps hash map bucket allocations scoped to the 🤖 Was this summary useful? React with 👍 or 👎 |
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.
Review completed. No suggestions at this time.
Comment augment review to trigger a new review at any time.
93ccc9e to
e14f024
Compare
a7e9d27 to
6580280
Compare
ebfd8b7 to
e16e6da
Compare
76c42dd to
b40e2ca
Compare
e16e6da to
82890e0
Compare
b40e2ca to
ad273fc
Compare
82890e0 to
59d55c1
Compare
ad273fc to
ea80a86
Compare
ea80a86 to
97088c1
Compare
97088c1 to
752a68c
Compare

🌟 What is the purpose of this PR?
Add
Interned::empty()for zero-length slices and useIdProducerinInternMap🔍 What does this change?
Interned::empty()method that returns a canonical empty interned sliceAtomicU32withIdProducer<T::Id>inInternMapfor ID generationLocalLockinstances withfast_hash_map()instead of usingdefault()intern_sliceto return the canonical empty slice for empty inputstest:miriscript to include the newstable_empty_slicetestPre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🛡 What tests cover this?
stable_empty_sliceto verify that empty slices share the same addressIdProducerchanges❓ How to test this?
cargo testto verify all tests passcargo miri nextest run -- stable_empty_sliceto verify the new empty slice functionality