Skip to content

Commit

Permalink
kv: store span by pointer in latch struct
Browse files Browse the repository at this point in the history
This pr changes the latch struct to store a pointer to span that is preallocated
on the heap. The new struct will reduce memory footprint of span by 40 bytes. One
side effect would be increase the probably of running into nil pointer issue
especially when spans pointer are not initialized and accessed properly.

Fixes: #114603
Release note: None.
  • Loading branch information
Eric.Yang authored and nvanbenschoten committed Nov 22, 2023
1 parent 1473312 commit 831c61d
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 13 deletions.
49 changes: 36 additions & 13 deletions pkg/kv/kvserver/spanlatch/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func Make(stopper *stop.Stopper, slowReqs *metric.Gauge) Manager {
type latch struct {
*signals
id uint64
span roachpb.Span
span *roachpb.Span
ts hlc.Timestamp
next, prev *latch // readSet linked-list.
}
Expand All @@ -109,13 +109,36 @@ func (la *latch) SafeFormat(w redact.SafePrinter, _ rune) {
//go:generate ../../../util/interval/generic/gen.sh *latch spanlatch

// Methods required by util/interval/generic type contract.
func (la *latch) ID() uint64 { return la.id }
func (la *latch) Key() []byte { return la.span.Key }
func (la *latch) EndKey() []byte { return la.span.EndKey }
func (la *latch) New() *latch { return new(latch) }
func (la *latch) SetID(v uint64) { la.id = v }
func (la *latch) SetKey(v []byte) { la.span.Key = v }
func (la *latch) SetEndKey(v []byte) { la.span.EndKey = v }
func (la *latch) New() *latch { return new(latch) }
func (la *latch) ID() uint64 { return la.id }
func (la *latch) SetID(v uint64) { la.id = v }
func (la *latch) Key() []byte {
if la.span == nil {
return nil
}
return la.span.Key
}
func (la *latch) EndKey() []byte {
if la.span == nil {
return nil
}
return la.span.EndKey
}
func (la *latch) SetKey(v []byte) {
la.initSpan()
la.span.Key = v
}
func (la *latch) SetEndKey(v []byte) {
la.initSpan()
la.span.EndKey = v
}

// initSpan lazily initializes the latch's span field. Only used in tests.
func (la *latch) initSpan() {
if la.span == nil {
la.span = new(roachpb.Span)
}
}

type signals struct {
done signal
Expand Down Expand Up @@ -198,7 +221,7 @@ func newGuard(spans *spanset.SpanSet, pp poison.Policy) *Guard {
ssLatches := latches[:n]
for i := range ssLatches {
latch := &latches[i]
latch.span = ss[i].Span
latch.span = &ss[i].Span
latch.signals = &guard.signals
latch.ts = ss[i].Timestamp
// latch.setID() in Manager.insert, under lock.
Expand Down Expand Up @@ -287,9 +310,9 @@ func (m *Manager) CheckOptimisticNoConflicts(lg *Guard, spans *spanset.SpanSet)
tr := &snap.trees[s]
for a := spanset.SpanAccess(0); a < spanset.NumSpanAccess; a++ {
ss := spans.GetSpans(a, s)
for _, sp := range ss {
search.span = sp.Span
search.ts = sp.Timestamp
for i := range ss {
search.span = &ss[i].Span
search.ts = ss[i].Timestamp
switch a {
case spanset.SpanReadOnly:
// Search for writes at equal or lower timestamps.
Expand Down Expand Up @@ -561,7 +584,7 @@ func (m *Manager) waitForSignal(
// ourselves anyway, so we don't need to self-poison.
switch pp {
case poison.Policy_Error:
return poison.NewPoisonedError(held.span, held.ts)
return poison.NewPoisonedError(*held.span, held.ts)
case poison.Policy_Wait:
log.Eventf(ctx, "encountered poisoned latch; continuing to wait")
wait.poison.signal()
Expand Down
8 changes: 8 additions & 0 deletions pkg/kv/kvserver/spanlatch/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"strings"
"testing"
"time"
"unsafe"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/poison"
Expand Down Expand Up @@ -681,6 +682,13 @@ func TestLatchManagerWaitFor(t *testing.T) {
m.Release(lg3)
}

// TestSizeOfLatch tests the size of the latch struct.
func TestSizeOfLatch(t *testing.T) {
var la latch
size := int(unsafe.Sizeof(la))
require.Equal(t, 56, size)
}

func BenchmarkLatchManagerReadOnlyMix(b *testing.B) {
for _, size := range []int{1, 4, 16, 64, 128, 256} {
b.Run(fmt.Sprintf("size=%d", size), func(b *testing.B) {
Expand Down

0 comments on commit 831c61d

Please sign in to comment.