From 537d9ac4ff791a66d37ee359989848b89b3277bc Mon Sep 17 00:00:00 2001
From: Shaun Patterson <shaunpatterson@gmail.com>
Date: Sun, 24 May 2026 15:44:01 -0400
Subject: [PATCH] perf(iterator): lazy-allocate Item.slice
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

newItem eagerly allocated a y.Slice wrapper per Item even though both
users of item.slice (yieldItemValue and prefetchValue) already nil-check
and lazy-init it before any access. Iterators that never read values
(KeyOnly, or AllVersions=true with PrefetchValues=false — e.g. dgraph's
posting list rollup walking ~8 versions per cache miss) were paying a
24-byte allocation per Item creation for a struct they never touched.

Drop the eager new(y.Slice) and rely on the existing lazy-init. Add a
regression test covering all three value-reading paths
(PrefetchValues=true, synchronous Item.Value, ValueCopy) to ensure the
lazy initialization stays correct.

BenchmarkRollupKeyIterator (Apple M4 Max, 5×3s):
  Before:  ~910 ns/op  873 B/op  18 allocs/op
  After:   ~855 ns/op  825 B/op  16 allocs/op

~6% faster, 48 B/op saved, 2 fewer allocations per iterator setup.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 iterator.go                 | 12 +++++-
 iterator_lazy_slice_test.go | 86 +++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 iterator_lazy_slice_test.go

diff --git a/iterator.go b/iterator.go
index f57cfa4c9..caeb814a9 100644
--- a/iterator.go
+++ b/iterator.go
@@ -34,7 +34,11 @@ type Item struct {
 	version   uint64
 	expiresAt uint64
 
-	slice *y.Slice // Used only during prefetching.
+	// slice is lazily allocated; only touch via yieldItemValue/prefetchValue,
+	// which nil-check and initialize on first use. Iterators that never read
+	// values (KeyOnly or PrefetchValues=false with no Item.Value/ValueCopy
+	// calls — e.g. dgraph's posting list rollup) skip this allocation entirely.
+	slice *y.Slice
 	next  *Item
 	txn   *Txn
 
@@ -506,7 +510,11 @@ func (txn *Txn) NewKeyIterator(key []byte, opt IteratorOptions) *Iterator {
 func (it *Iterator) newItem() *Item {
 	item := it.waste.pop()
 	if item == nil {
-		item = &Item{slice: new(y.Slice), txn: it.txn}
+		// Skip the eager new(y.Slice): yieldItemValue lazy-allocates slice
+		// before any actual use. Iterators that never read values (KeyOnly,
+		// or AllVersions=true with PrefetchValues=false like dgraph's posting
+		// list rollup) never touch slice, so the wrapper is pure overhead.
+		item = &Item{txn: it.txn}
 	}
 	return item
 }
diff --git a/iterator_lazy_slice_test.go b/iterator_lazy_slice_test.go
new file mode 100644
index 000000000..98ae59a0c
--- /dev/null
+++ b/iterator_lazy_slice_test.go
@@ -0,0 +1,86 @@
+/*
+ * SPDX-FileCopyrightText: © 2017-2025 Dgraph Labs, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package badger
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestRegressionLazyItemSliceValueRead exercises Item.Value/ValueCopy on
+// items produced by an iterator that uses the lazy-slice allocation path
+// (newItem no longer eagerly allocates y.Slice). yieldItemValue and
+// prefetchValue must lazy-init item.slice on first use; if they don't,
+// item.slice.Resize() will nil-deref.
+//
+// Covers three configurations that previously relied on the eager
+// allocation: PrefetchValues=true (prefetch goroutine), PrefetchValues=false
+// + Item.Value() (synchronous read), and ValueCopy on a lazy item.
+func TestRegressionLazyItemSliceValueRead(t *testing.T) {
+	runBadgerTest(t, nil, func(t *testing.T, db *DB) {
+		// Two keys with distinct, non-empty values so the value path is
+		// actually traversed (deletes/empty values exit yieldItemValue
+		// before touching slice).
+		txnSet(t, db, []byte("k1"), []byte("value-one"), 0)
+		txnSet(t, db, []byte("k2"), []byte("value-two-larger"), 0)
+
+		// Variant A: PrefetchValues=true triggers the prefetch goroutine
+		// path, which calls yieldItemValue → item.slice.Resize.
+		require.NoError(t, db.View(func(txn *Txn) error {
+			opt := DefaultIteratorOptions
+			opt.PrefetchValues = true
+			it := txn.NewIterator(opt)
+			defer it.Close()
+			count := 0
+			for it.Rewind(); it.Valid(); it.Next() {
+				item := it.Item()
+				require.NoError(t, item.Value(func(val []byte) error {
+					require.NotEmpty(t, val, "prefetched value must be non-empty")
+					return nil
+				}))
+				count++
+			}
+			require.Equal(t, 2, count)
+			return nil
+		}))
+
+		// Variant B: PrefetchValues=false + synchronous Item.Value(). Item
+		// is produced with slice=nil; yieldItemValue must initialize it.
+		require.NoError(t, db.View(func(txn *Txn) error {
+			opt := DefaultIteratorOptions
+			opt.PrefetchValues = false
+			it := txn.NewIterator(opt)
+			defer it.Close()
+			count := 0
+			for it.Rewind(); it.Valid(); it.Next() {
+				item := it.Item()
+				require.NoError(t, item.Value(func(val []byte) error {
+					require.NotEmpty(t, val)
+					return nil
+				}))
+				count++
+			}
+			require.Equal(t, 2, count)
+			return nil
+		}))
+
+		// Variant C: ValueCopy on a lazy item — separate code path through
+		// yieldItemValue → SafeCopy.
+		require.NoError(t, db.View(func(txn *Txn) error {
+			opt := DefaultIteratorOptions
+			opt.PrefetchValues = false
+			it := txn.NewIterator(opt)
+			defer it.Close()
+			it.Rewind()
+			require.True(t, it.Valid())
+			buf, err := it.Item().ValueCopy(nil)
+			require.NoError(t, err)
+			require.NotEmpty(t, buf)
+			return nil
+		}))
+	})
+}