From f3983315c412fb57f3402a614a5ddc3e48368089 Mon Sep 17 00:00:00 2001 From: Shaun Patterson Date: Thu, 28 May 2026 14:14:02 -0400 Subject: [PATCH] perf(posting): avoid redundant proto.Clone in setMutationAfterCommit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When rebuilding committedEntries, copy the *pb.PostingList values by reference instead of proto.Clone. Per MutableLayer's documented invariant, committed entries are immutable once inserted and MutableLayer.clone() already shares the map by reference, so the deep clone was redundant. Only the map itself must be freshly allocated. BenchmarkSetMutationAfterCommit (priorEntries 1..256): sec/op 825µs -> 174µs (-78.9%, p=0.002) B/op 1.65Mi -> 304Ki (-82.0%, p=0.002) allocs/op 15.26k -> 40 (-99.74%, p=0.002) --- posting/list.go | 7 ++++++- posting/list_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/posting/list.go b/posting/list.go index 1c0c7a0fc55..a364136ade6 100644 --- a/posting/list.go +++ b/posting/list.go @@ -988,9 +988,14 @@ func (l *List) setMutationAfterCommit(startTs, commitTs uint64, pl *pb.PostingLi } if refresh { + // The committedEntries map may be aliased by other transactions' MutableLayer + // clones (see MutableLayer.clone), so we cannot mutate it in place. We do however + // copy the map by reference of its values: per the type's invariant (see comment + // above MutableLayer), committed entry *pb.PostingLists are treated as immutable + // once inserted, so a shallow copy is sufficient. newMap := make(map[uint64]*pb.PostingList, l.mutationMap.len()) for k, v := range l.mutationMap.committedEntries { - newMap[k] = proto.Clone(v).(*pb.PostingList) + newMap[k] = v } newMap[commitTs] = pl l.mutationMap.committedEntries = newMap diff --git a/posting/list_test.go b/posting/list_test.go index 79bce4a1879..39bdf3c9ad3 100644 --- a/posting/list_test.go +++ b/posting/list_test.go @@ -1834,3 +1834,37 @@ func BenchmarkAddMutations(b *testing.B) { } } } + +// BenchmarkSetMutationAfterCommit exercises the commit hot path that builds a +// fresh committedEntries map. This is the path UpdateCachedKeys → updateItemInCache +// → setMutationAfterCommit(refresh=true) walks for every committed transaction +// against a cached List. Allocation behavior here directly drives GC pressure +// on heavily mutated lists. +func BenchmarkSetMutationAfterCommit(b *testing.B) { + for _, prior := range []int{1, 4, 32, 256} { + b.Run(fmt.Sprintf("priorEntries=%d", prior), func(b *testing.B) { + key := x.DataKey(x.AttrInRootNamespace("bench-commit"), uint64(prior)) + l, err := GetNoStore(key, math.MaxUint64) + if err != nil { + b.Fatal(err) + } + l.mutationMap = newMutableLayer() + // Pre-populate committedEntries with `prior` versions to simulate a + // list that has been written many times. + for i := 0; i < prior; i++ { + pl := &pb.PostingList{ + Postings: []*pb.Posting{{Uid: uint64(i + 1), Op: Set}}, + } + l.mutationMap.committedEntries[uint64(i+1)] = pl + } + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + pl := &pb.PostingList{ + Postings: []*pb.Posting{{Uid: uint64(i + 1000), Op: Set}}, + } + l.setMutationAfterCommit(uint64(prior+i+1), uint64(prior+i+2), pl, true) + } + }) + } +}