Repository navigation
Merged
Conversation
…ext/Prev, Verify ref copy - Cursor.Upsert's in-place path kept the old key when the new one compared equal and reported the new key as PrevKey; it now replaces the key and reports the real previous one. Tested with keys compared by id and an augmentation that depends on the key. - The interval augmentation started from the zero endpoint as its bound: wrong for negative endpoints and a panic for comparators that cannot take the zero value. subtreeBound now has an explicit unset state and findUpperBound reports emptiness. Tested with pointer endpoints and negative spans in the property test. - interval.Iterator.Next and Prev end an overlap scan, as the seeks do. - Verify copies only the fields the Updater reads instead of the whole node, whose reference count another goroutine may be changing atomically. A race test verifies a snapshot while the original is cloned, written and cleared. - SeekWhere's documented contract now matches its single-descent algorithm: the predicate must be exact for spans. The aug package doc states that maps must not be copied by value and that keys, values and augmentations are shallow-copied. Each new test fails on v0.2.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ints and TieBreak, config copy, Verify equality - A split, merge or rebalance recomputes the restructured node and reports a change upward, so shape-dependent augmentations (node counts, heights) stay correct, including the merge caused by deleting an absent key. Tested with a node-counting Updater; on v0.2.0 the reviewer's degree-2 example leaves the root at 3 instead of 4. - Reference counts are int64. - An interval whose end is not after its start is a point, so leaf matching and subtree pruning agree wherever it sits; reversed intervals are points too. Tested alone in a leaf, after many splits, and reversed. - interval.Bounds.TieBreak replaces CompareIntervals and is consulted only for equal start keys, since overlap searches require start order. Tested with a tie-breaker preferring ends. - LowLevelIterator.Config returns a copy. - Verify compares augmentations with the Updater's Equal when it implements Equaler; MonoidUpdater forwards the Monoid's. Tested with a NaN-propagating maximum. - interval.Cursor, FreeList and NewFreeList are nameable; the package documents its overlap cost honestly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ies, CommutativeMonoid Iterators and cursors: every positioning method (First, Last, Next, Prev, the seeks, SeekNth) returns whether the iterator is now at an entry, so a range loop is "for ok := it.SeekGE(lo); ok && ...; ok = it.Next()". SeekGE and SeekLT no longer report whether the sought key exists; SeekExact does. Cursor.Upsert returns the replaced key and value like Map.Upsert, Rekey returns the entry it displaced, and Delete documents the deletion loop. Sets: Get(probe) returns the stored item equal to a probe and Delete returns the removed item, since comparator equality defines identity. SetIterator and SetCursor are their own types with Item and no Value, SetValue or phantom value arguments. The btree package documents the comparison contract and NewOrdered's NaN limitation. interval: queries are Spans (HalfOpen, Point) separate from the stored type; Overlaps(span) returns an OverlapIterator whose Next always means the next overlap and Iterator has no overlap mode; Bounds.HasEnd defaults to "every interval has an end, and one whose end is not after its start is a point" instead of the zero-end convention. Empty and reversed query spans overlap nothing. aug: Monoid and Group are CommutativeMonoid and CommutativeGroup with an Identity method, and the zero value is no longer assumed to be the identity. SeekWhere(prefix, contribution) is SeekPrefix(inclusivePrefix) with a predicate that turns true at most once, returning the prefix and whether an entry was found. Node.Aug and ChildAug return values and Node.SetAug is for Updaters, so traversal cannot modify augmentations; LowLevelIterator.Config already returns a copy. MonoidMap is a defined type over Map with no exported embedded field. New panics on a nil comparison function, the zero Map is documented as unusable, and the package states one ownership contract for every collection. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 of 3 tasks
ajwerner
changed the title
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…usable overlap iterator - Shape-dependent augmentations opt into structural events by implementing Restructurer; the tree no longer recomputes every node after a split, merge or rebalance for augmentations that depend only on the entries, which had made the interval bound recompute run on every merge (Delete +30-70% in the interval benchmarks). - Nodes start from the monoid's Identity, fresh and recycled, so a monoid whose identity is not the zero value (a product) aggregates correctly. - interval.Set gets Get, an item-returning Delete, SetIterator, SetCursor and SetOverlapIterator with Item and no values. The default TieBreak treats every representation of a point at the same key as the same item. Bounds.CompareIntervals is back as an optional single-call comparator that must order by start, which Verify now checks. - OverlapIterator.Seek restarts a scan on an existing iterator, so a query no longer costs a fresh 300-byte iterator; the ported benchmarks use it. - Config.Compare has a value receiver; the interval updater stores its bound without a deferred closure; Next and Prev return their validity from the position they already know; Map.Iterator's doc says to reposition rather than recreate after a mutation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ext fast path - The point-or-range decision is HasEnd's alone: the default HasEnd normalises (an end not after the start makes a point) and a custom HasEnd is trusted, so computing a bound costs no comparison; an interval a custom HasEnd claims an end for that does not end after it starts panics at insertion or replacement. - OverlapIterator caches the updater instead of copying the Config and type-asserting on every step. - Iterator.Next returns without settling when the next entry is in the same leaf. Against v0.2.0 the interval and iteration benchmarks are now within noise or ahead (geomean -1.3%); the rest of the suite was already at parity. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…descents Set.Get had built an iterator and sought; it now uses Map.Lookup, a Get that also returns the stored key, and Contains uses Get directly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Summary
One PR for all three review rounds on
v0.2.0and the API design pass. Breaking on top ofv0.2.0; the changelog lists every renamed or re-typed symbol. Tag after merge:v0.3.0.Correctness fixes (each with a test; those runnable against
v0.2.0fail there)Cursor.Upsertwith a distinct but equal key replaced only the value and misreportedPrevKey.Verifycopied the atomic reference count non-atomically and wrote augmentations of shared nodes.Restructurerto be told (the reviewer's degree-2 example: root count 3 instead of 4). Entry-only augmentations pay nothing.Identity, fresh and recycled; a product monoid (identity 1) aggregated to 0 before.HasEnd; a customHasEndthat claims an end for one is rejected at insertion. The default tie-break treats every representation of a point at a key as the same item.Verifyuses the Updater'sEqualer(NaN-safe aggregates) and, for interval trees, checks that intervals are ordered by start.API pass
SeekExactanswers "does the key exist".Set.Get(probe),Map.Lookup(k), item-returningSet.Delete, cursorUpsert/Rekeyreturning displaced entries; the comparison contract andNewOrdered's NaN limit are documented.SetIterator/SetCursorforbtree,orderstatandinterval(plusSetOverlapIterator) withItemand noValue,SetValueor phantom arguments.Cursor.Deletedocuments the deletion loop.Spans (HalfOpen,Point) separate from the stored type;Overlaps(span)returns anOverlapIteratorwhoseNextis always the next overlap and whoseSeekreuses it; the plainIteratorhas no overlap mode.Bounds.TieBreakorders equal starts;CompareIntervalsremains as an optional single-call comparator that must order by start (checked byVerify).MonoidMaphas no exported embedded field;Node.Aug/ChildAugreturn values andSetAugis for Updaters;LowLevelIterator.Configreturns a copy andConfig.Comparehas a value receiver.CommutativeMonoid/CommutativeGroupwithIdentity;SeekPrefix(inclusivePrefix)with a once-flipping predicate replacesSeekWhere.Newpanics on a nil comparison function; the zeroMapis documented unusable; one ownership contract in theaugdocs;Map.Iterator's doc says to reposition after a mutation.Not done: type-checked free-list options. A
btree.FreeList[int, int]can still be passed to abtree.New[int, string]and only fails at construction, because functional options cannot be typed and inferable in Go without type arguments at every call site. A typed config struct would fix it at the cost of every constructor call; left as an accepted trade-off.Performance
In-repo benchmarks against
v0.2.0in the same session (benchstat, 4 runs): non-interval benchmarks at parity or better; the interval package, which regressed up to 3× in the first cut of the API pass (per-query iterator construction, a recompute on every merge, an extra comparison per bound), is back to parity or ahead (geomean -1.3%) afterOverlapIterator.Seek, opt-inRestructurerandHasEnd-only bounds. The google/btree harness comparison was run interleaved (five alternations ofv0.2.0and the branch, compiled binaries, same machine state) because the machine drifted mid-session. The google/btree control rows, identical code on both sides, moved by up to ±15% between columns, which is the noise floor for that run; within it the branch is at parity (geomean -5%), with no row outside the control's own swing. Same-session in-repo benchstat (quieter machine, 4 runs) remains the reliable A/B: parity or better throughout.Test plan
go test -race ./...on Go 1.26.8 and 1.27.1v0.2.0for the in-repo benchmarks🤖 Generated with Claude Code