BE-607: HashQL: Refactor ModuleRegistry into builder/immutable split with allocator-generic stdlib construction - #8887
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryHigh Risk Overview Standard library registration drops the nested Resolution and diagnostics: root import wiring reads Symbols: stdlib paths and names move to Unrelated: Reviewed by Cursor Bugbot for commit 5a8147e. Bugbot is set up for automated code reviews on this repo. Configure here. |
6696c2c to
5d51d92
Compare
5d51d92 to
f62be24
Compare
f62be24 to
558d30c
Compare
558d30c to
d08cc92
Compare
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
d08cc92 to
5014ae0
Compare
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), |
| pub const fn from_db_name(name: &str) -> Option<Self> { | ||
| match name.as_bytes() { | ||
| b"a14" => Some(Self::M1), | ||
| b"a15" => Some(Self::M2), | ||
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), |
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
| b"a14" => Some(Self::M1), | ||
| b"a15" => Some(Self::M2), | ||
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), |
| @@ -277,27 +322,15 @@ impl<'heap> ModuleRegistry<'heap> { | |||
| /// This method requires allocation for the traversal state: | |||
| /// - A vector for the module exploration stack | |||
| /// - A hash set for tracking visited modules | |||
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), |
| /// This method requires allocation for the traversal state: | ||
| /// - A vector for the module exploration stack | ||
| /// - A hash set for tracking visited modules |
| b"a14" => Some(Self::M1), | ||
| b"a15" => Some(Self::M2), | ||
| b"a16" | b"as1" | b"as2" | b"as3" => Some(Self::M3), | ||
| b"as4" | b"as4-1" | b"as4-2" => Some(Self::M4), | ||
| b"as5" | b"as5-1" | b"as5-2" => Some(Self::M5), | ||
| b"as5" | b"as5-2" => Some(Self::M5), | ||
| _ => None, |
| /// Looks up a root-level module by name. | ||
| #[must_use] | ||
| pub fn find_by_name(&self, name: Symbol<'heap>) -> Option<Module<'heap>> { | ||
| let id = self.root.get(&name).copied()?; | ||
|
|
||
| Some(self.modules[id]) | ||
| } |
| if next == ModuleId::ROOT { | ||
| return None; | ||
| } | ||
|
|
||
| let module = registry.modules.index(next); | ||
| let module = registry.modules[next]; | ||
| next = module.parent; | ||
|
|
||
| Some(module) | ||
| }) |

🌟 What is the purpose of this PR?
Replaces the
StandardLibrarytype (12.9KB on the stack due to nestedSmallVec) with a flatModuleCachebacked byMaybeUninitslots and aFiniteBitSet, and restructuresModuleRegistryconstruction around a builder pattern (PartialModuleRegistry). Reduces stdlib construction cost by ~20% in instructions and ~60% in L1D cache misses.🔍 What does this change?
Module cache and construction:
SmallVec<(TypeId, ModuleDef)>inStandardLibrarywith a flatModuleCacheusingBox<[MaybeUninit<ModuleDef>]>andFiniteBitSet<CacheId, u64>for initialization trackingStandardLibraryintoStandardLibrary(coordinator),StandardLibraryContext(passed todefinemethods), andModuleCache(shared across modules)CacheIdenum with one variant per stdlib module in preorder traversal order, replacing runtimeTypeIdlinear scan with compile-time indicesModuleDeftracksemitted_lenfor exact output capacity when building item slicesmatchinbuild()instead of[Option<ItemKind>; 2].flatten()for item emissionInternSet) since module item slices are provably unique byModuleIdownershipRegistry restructuring:
PartialModuleRegistryas a mutable builder that collects modules during constructionModuleRegistryis now immutable after construction, backed by a contiguousIdSliceinstead ofInternMapRwLockfrom the root namespace (mutable access during construction, shared reference after)ModuleRegistry::builder()test helper andPartialModuleRegistry::insert_root_module()for tests that inject custom modulesSymbol and type cleanup:
heap.intern_symbol("...")calls in stdlib modules with pre-internedsym::constants.opaque("::...")calls withsym::path::nested constantsInterned::empty()for zero-genericdecl!expansions and special form typesModuleDefparameterized by allocatorSfor bump-scope 2.x supportBenchmark results (Apple Silicon, bump-scope 2.x):
Pre-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:
🐾 Next steps
Potential further optimizations identified but not implemented:
TypeIds (number, boolean, etc.) onTypeBuilderto avoid repeated intern round-tripsTypeDefs in modules likemathandspecial_formdecl!macro🛡 What tests cover this?
hashql-coretest suite (880 unit + 326 integration + 20 doc tests) all passmodule::namespaceandmodule::resolvertests exercise custom module construction through the newPartialModuleRegistrybuilder API❓ How to test this?
cargo test --package hashql-core