From fad07b1c83fd5ada9ba141cabe7105f398adf9c4 Mon Sep 17 00:00:00 2001 From: Shaun Patterson Date: Sat, 23 May 2026 13:57:55 -0400 Subject: [PATCH 1/2] perf(table): skip redundant first bytes.Equal in MergeIterator.Next MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invariant: every public method that leaves MergeIterator Valid ends with setCurrent(), which sets curKey = small.key. So on entry to Next(), curKey == small.key byte-for-byte, and the first iteration of the original duplicate-skip loop's bytes.Equal check is always true — we always need to advance once before checking for duplicates. Hoist the first small.next() + fix() out of the loop and keep checking for duplicates only on subsequent iterations. Saves one bytes.Equal call per emitted key on the merged-read hot path. Existing TestMergeDuplicates (forward + reverse) and TestMergeIterator* cover this code path. Benchmark impact (M4 Max, median of 3 runs): - BenchmarkReadMerged: -2.4% (~523M -> ~510M ns) --- table/merge_iterator.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/table/merge_iterator.go b/table/merge_iterator.go index b42d79e03..14bafced8 100644 --- a/table/merge_iterator.go +++ b/table/merge_iterator.go @@ -139,10 +139,17 @@ func (mi *MergeIterator) swapSmall() { // Next returns the next element. If it is the same as the current key, ignore it. func (mi *MergeIterator) Next() { - for mi.Valid() { - if !bytes.Equal(mi.small.key, mi.curKey) { - break - } + if !mi.Valid() { + return + } + // Invariant on entry: mi.small.key == mi.curKey byte-for-byte, since every + // public method (Rewind/Seek/Next) that leaves the iterator Valid ends by + // calling setCurrent(). Skip the always-true first comparison and advance + // unconditionally; then keep advancing while the new small.key duplicates + // the saved curKey (only possible from the OTHER child iterator). + mi.small.next() + mi.fix() + for mi.Valid() && bytes.Equal(mi.small.key, mi.curKey) { mi.small.next() mi.fix() } From ff9c5e58eadf977f8182021173176bc6e988547d Mon Sep 17 00:00:00 2001 From: Shaun Patterson Date: Mon, 25 May 2026 21:47:34 -0400 Subject: [PATCH 2/2] test(table): cover Next() early-return on invalid MergeIterator Calling Next() after a Seek past the end (iterator already invalid) must be a safe no-op. The early-return guard at the top of Next() previously had no coverage. Add a focused regression that Seeks past the end then calls Next(), verifying no panic and Valid()=false. Co-Authored-By: Claude Opus 4.7 --- table/merge_iterator_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/table/merge_iterator_test.go b/table/merge_iterator_test.go index c19a3dc6f..370f25345 100644 --- a/table/merge_iterator_test.go +++ b/table/merge_iterator_test.go @@ -273,6 +273,22 @@ func TestMergeIteratorSeekInvalidReversed(t *testing.T) { closeAndCheck(t, mergeIt, 4) } +// TestMergeIteratorNextOnInvalid exercises the early-return guard in Next() +// for the case where Next is called on an iterator that has already become +// invalid (Seek past the end). The call must be a safe no-op. +func TestMergeIteratorNextOnInvalid(t *testing.T) { + it1 := newSimpleIterator([]string{"1", "3"}, []string{"a1", "a3"}, false) + it2 := newSimpleIterator([]string{"2", "4"}, []string{"b2", "b4"}, false) + mergeIt := NewMergeIterator([]y.Iterator{it1, it2}, false) + // Seek past the end so the iterator is invalid. + mergeIt.Seek([]byte("z")) + require.False(t, mergeIt.Valid()) + // Next on an invalid iterator must not panic and must remain invalid. + mergeIt.Next() + require.False(t, mergeIt.Valid()) + closeAndCheck(t, mergeIt, 2) +} + func TestMergeIteratorDuplicate(t *testing.T) { it1 := newSimpleIterator([]string{"0", "1", "2"}, []string{"a0", "a1", "a2"}, false) it2 := newSimpleIterator([]string{"1", "3"}, []string{"b1", "b3"}, false)