collections: drop byOff map + alias offsets; block zones/strDict maps -> slices - #222
Merged
Conversation
…rena-offset map Two more per-segment columnar-metadata copies the heap profile flagged, no on-disk format change. byOff: colNative kept a resident map[arenaOffset]index -- one entry per record per columnarized segment, ~164MB of live heap on a production archive -- for an O(1) lookup. The offset map is already in hand and strictly ascending (records append in order), so an O(log n) binary search (indexOf) replaces it; the map is removed. offs: colSegment.offs []uint32 was materialized from the section on every open. It is packed little-endian u32 on disk, so it becomes offsB []byte that ALIASES the mmap on a reopened segment (zero heap; a built one holds a compact buffer), read via offAt/offsLen -- the same aliasing #219 applied to the block offset arrays. Marshal output is byte-identical; readU32s is gone, packU32s added for the build path. ~196MB off the resident plateau, and byOff no longer scales with record count. TestColSegOffsAliasAndIndexOf covers the aliased read-back and indexOf (present -> index; below/between/above -> not-found). Full collections suite green (both stores). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ump) The last per-block columnar-metadata maps. A block kept zones (map[int]blockZone, ~174MB in the profile) and strDict (map[int]strDictField, ~156MB) -- one map each per block over ~100K+ blocks, whose bucket overhead was ~2x the entries. A block-format bump to alias them would be data loss on columnar-native segments (their schema'd attributes live only in the block payload, so rejecting an old section drops the only copy) and would need dual-format read to be safe. But the map overhead is a representation choice, not on-disk: parse the SAME on-disk entries into a sorted-by-idx slice and binary-search them (zone(), strDictOf()). Byte- identical marshal, so every existing segment gets the smaller form immediately with no migration. seg.zones (the per-segment zone map keyed by attr id) is unrelated and untouched. TestColBlockLayoutSharedAcrossBlocks extended: both slices sorted, every entry findable via its accessor, fixture non-vacuous. Full collections suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on
main. Reopens two changes that were lost: #220 and #221 were merged into feature-branch bases instead ofmain, so their commits never reachedmain. This carries both, offmain, in one PR (they are sequential — each edits the same columnar-metadata code in order — so they ship together rather than as two dependent PRs).Follows #219 (already on main). Removes the last of the growing per-block/per-segment columnar-metadata heap. No on-disk format change; byte-identical marshal; applies to existing segments immediately.
1. Drop the per-segment
byOffmap; alias the arena-offset map (was #220, ~196 MB)colNativekept a residentmap[arenaOffset]index, one entry per record per columnarized segment. The offset map is already in hand and strictly ascending, so an O(log n) binary search (indexOf) replaces it; the map is removed.colSegment.offs []uint32→offsB []bytealiasing the mmap on reopen (read viaoffAt/offsLen).2. Block
zones+strDictmaps → sorted slices (was #221, ~150–180 MB)map[int]blockZone/map[int]strDictField→ sorted-by-idx slices, binary-searched (zone,strDictOf). A Go map of a few dozen small entries carries ~2× its data in overhead, ×100K+ blocks.Tests
TestColSegOffsAliasAndIndexOf(aliased read-back +indexOfpresent/absent) and an extendedTestColBlockLayoutSharedAcrossBlocks(offset arrays packed u32; zones/strDict sorted, every entry findable, fixture non-vacuous). Fullcollectionssuite green on both stores.🤖 Generated with Claude Code