Skip to content

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Dec 28, 2025

🌟 What is the purpose of this PR?

Add Interned::empty() for zero-length slices and use IdProducer in InternMap

The previous version of this PR was about switching the allocator for interning, I found that there was no benefit to do so, hence another optimization: canonical empty slices (something that needed to be done either way for correctness) has been implemented. Giving a ~5-12% boost in places that mostly deal with empty collections, such as MIR passes.

🔍 What does this change?

  • Adds Interned::empty() method that returns a canonical empty interned slice
  • Replaces AtomicU32 with IdProducer<T::Id> in InternMap for ID generation
  • Initializes LocalLock instances with fast_hash_map() instead of using default()
  • Optimizes intern_slice to return the canonical empty slice for empty inputs
  • Adds a test for the stable empty slice functionality
  • Updates the test:miri script to include the new stable_empty_slice test

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

🛡 What tests cover this?

  • Added a new test stable_empty_slice to verify that empty slices share the same address
  • Existing tests cover the IdProducer changes

❓ How to test this?

  1. Run cargo test to verify all tests pass
  2. Run cargo miri nextest run -- stable_empty_slice to verify the new empty slice functionality
  3. Verify that interning empty slices returns the same reference regardless of element type

@vercel vercel bot temporarily deployed to Preview – petrinaut December 28, 2025 10:27 Inactive
@cursor
Copy link

cursor bot commented Dec 28, 2025

PR Summary

Introduces canonical empty interned slices and simplifies ID generation and map initialization.

  • Adds Interned::empty() for slices and updates InternSet::intern_slice to return the canonical empty slice for empty inputs
  • Replaces AtomicU32 with IdProducer<T::Id> in InternMap and updates next_id() accordingly
  • Initializes LocalLock maps with fast_hash_map() in InternMap and InternSet
  • Adds stable_empty_slice test and includes it in the test:miri script

Written by Cursor Bugbot for commit 752a68c. This will update automatically on new commits. Configure here.

Copy link
Member Author

indietyp commented Dec 28, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codecov
Copy link

codecov bot commented Dec 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.36%. Comparing base (bf829ac) to head (752a68c).
⚠️ Report is 2 commits behind head on main.

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              
Flag Coverage Δ
rust.hash-graph-api 2.89% <ø> (ø)
rust.hashql-compiletest 46.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 28, 2025

Merging this PR will improve performance by 13.31%

⚡ 1 improved benchmark
✅ 13 untouched benchmarks
🗄️ 15 archived benchmarks run1

Performance Changes

Benchmark BASE HEAD Efficiency
diamond 7.2 µs 6.4 µs +13.31%

Comparing bm/be-262-hashql-use-heap-to-store-interners (752a68c) with bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping (82890e0)2

Open in CodSpeed

Footnotes

  1. 15 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them.

  2. No successful run was found on bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping (226e70c) during the generation of this report, so 5e19b4d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@augmentcode
Copy link

augmentcode bot commented Dec 28, 2025

🤖 Augment PR Summary

Summary: Refactors HashQL interning collections to allocate their backing hash tables in the arena Heap and to centralize sequential ID generation.

Changes:

  • InternMap: replace the manual AtomicU32 counter with IdProducer<T::Id> and generate IDs via IdProducer::next()
  • InternMap: build inner/lookup via fast_hash_map_in / fast_hash_map_with_capacity_in, and carry &Heap as the map allocator parameter
  • InternSet: allocate the internal map via the same *_in helpers and update the map type to include the allocator parameter

Technical Notes: This keeps hash map bucket allocations scoped to the Heap lifetime and removes duplicated atomic counter logic in favor of the shared ID utility.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a 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.

@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from 93ccc9e to e14f024 Compare December 28, 2025 11:17
@indietyp indietyp force-pushed the bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping branch from a7e9d27 to 6580280 Compare December 28, 2025 11:17
@vercel vercel bot temporarily deployed to Preview – petrinaut December 28, 2025 11:18 Inactive
@indietyp indietyp force-pushed the bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping branch from ebfd8b7 to e16e6da Compare January 15, 2026 15:22
@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from 76c42dd to b40e2ca Compare January 15, 2026 15:22
@indietyp indietyp force-pushed the bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping branch from e16e6da to 82890e0 Compare January 16, 2026 10:38
@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from b40e2ca to ad273fc Compare January 16, 2026 10:38
@indietyp indietyp changed the base branch from bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping to graphite-base/8223 January 16, 2026 12:46
@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from ad273fc to ea80a86 Compare January 16, 2026 12:47
@github-actions github-actions bot added the area/deps Relates to third-party dependencies (area) label Jan 16, 2026
@graphite-app graphite-app bot changed the base branch from graphite-base/8223 to main January 16, 2026 12:47
@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from ea80a86 to 97088c1 Compare January 16, 2026 12:47
@vercel vercel bot temporarily deployed to Preview – petrinaut January 16, 2026 12:47 Inactive
@indietyp indietyp changed the base branch from main to graphite-base/8223 January 16, 2026 13:45
@indietyp indietyp force-pushed the bm/be-262-hashql-use-heap-to-store-interners branch from 97088c1 to 752a68c Compare January 16, 2026 13:46
@github-actions github-actions bot removed the area/deps Relates to third-party dependencies (area) label Jan 16, 2026
@indietyp indietyp changed the base branch from graphite-base/8223 to bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping January 16, 2026 13:46
Base automatically changed from bm/be-260-hashql-switch-internal-allocator-implementation-to-scoping to main January 16, 2026 14:20
@indietyp indietyp added this pull request to the merge queue Jan 16, 2026
Merged via the queue into main with commit 7c64ae2 Jan 16, 2026
96 of 115 checks passed
@indietyp indietyp deleted the bm/be-262-hashql-use-heap-to-store-interners branch January 16, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team

Development

Successfully merging this pull request may close these issues.

3 participants