Skip to content

Commit ff76b95

Browse files
committed
kvserver/gc,rowcontainer: fix misuse of ByteAllocator API
`bufalloc.ByteAllocator.Copy` has an option to allocate extra capacity, meaning that it copies the passed-in byte slice into a new memory region while also adding extra capacity to be utilized by caller. Crucially, that extra capacity lives in the returned byte slice containing the copy. We use this feature in two places throughout the codebase, and in both places it was used incorrectly, resulting in a wasteful allocation. Consider the following setup: k = [1, 2], v = [3, 4, 5, 6] With invocation like `alloc, k = alloc.Copy(k, len(v))`, we would get len(k) = 2, cap(k) = 6 However, in both spots we copied `v` via `alloc, v = alloc.Copy(v, 0)`, which would actually create another allocation. This commit fixes this oversight (both were introduced in 2022 by different authors), and the following commit will clean up the API to avoid possible confusion. Release note: None
1 parent 0e90087 commit ff76b95

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

pkg/kv/kvserver/gc/gc_iterator.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,12 @@ func (b *gcIteratorRingBuf) pushBack(
239239
b.allocs[i] = b.allocs[i].Truncate()
240240
b.allocs[i], k.Key = b.allocs[i].Copy(k.Key, len(metaValue))
241241
if len(metaValue) > 0 {
242-
b.allocs[i], metaValue = b.allocs[i].Copy(metaValue, 0)
242+
// k.Key now contains the capacity for copy of metaValue - separate it
243+
// out.
244+
buf := k.Key[len(k.Key):cap(k.Key)]
245+
k.Key = k.Key[:len(k.Key):len(k.Key)]
246+
copy(buf, metaValue)
247+
metaValue = buf
243248
}
244249
b.buf[i] = mvccKeyValue{
245250
key: k,

pkg/sql/rowcontainer/disk_row_container.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,11 @@ func (r *diskRowIterator) EncRow() (rowenc.EncDatumRow, error) {
524524
// reuse the same byte slice across EncRow() calls because it would lead to
525525
// modification of the EncDatums (which is not allowed).
526526
r.rowBuf, k = r.rowBuf.Copy(k, len(v))
527-
r.rowBuf, v = r.rowBuf.Copy(v, 0 /* extraCap */)
527+
// k now contains the capacity for copy of v - separate it out.
528+
buf := k[len(k):cap(k)]
529+
k = k[:len(k):len(k)]
530+
copy(buf, v)
531+
v = buf
528532

529533
for i, orderInfo := range r.rowContainer.ordering {
530534
// Types with composite key encodings are decoded from the value.

0 commit comments

Comments
 (0)