Skip to content

Commit

Permalink
kv: return RangeIterator by value from constructor
Browse files Browse the repository at this point in the history
This commit changes `RangeIterator`'s constructor to return the struct
by value. The methods on `RangeIterator` continue to use a pointer
receiver, but this allows the struct to remain on the stack in some
cases and to be embedded in larger structs (e.g. `spanResolverIterator`)
in others. As a result, it avoids some heap allocations.

```
name                   old time/op    new time/op    delta
KV/Scan/SQL/rows=1-10    92.5µs ± 4%    94.4µs ± 5%    ~     (p=0.211 n=9+10)

name                   old alloc/op   new alloc/op   delta
KV/Scan/SQL/rows=1-10    20.1kB ± 0%    20.1kB ± 0%    ~     (p=0.782 n=10+10)

name                   old allocs/op  new allocs/op  delta
KV/Scan/SQL/rows=1-10       245 ± 0%       244 ± 0%  -0.41%  (p=0.000 n=10+8)
```
  • Loading branch information
nvanbenschoten committed Dec 30, 2021
1 parent cc22c45 commit d54e0dd
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 16 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/kvfeed/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func allRangeSpans(

ranges := make([]roachpb.Span, 0, len(spans))

it := kvcoord.NewRangeIterator(ds)
it := kvcoord.MakeRangeIterator(ds)

for i := range spans {
rSpan, err := keys.SpanAddr(spans[i])
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvclient/kvcoord/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ func (ds *DistSender) getNodeDescriptor() *roachpb.NodeDescriptor {
// CountRanges returns the number of ranges that encompass the given key span.
func (ds *DistSender) CountRanges(ctx context.Context, rs roachpb.RSpan) (int64, error) {
var count int64
ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
for ri.Seek(ctx, rs.Key, Ascending); ri.Valid(); ri.Next(ctx) {
count++
if !ri.NeedAnother(rs) {
Expand Down Expand Up @@ -1165,7 +1165,7 @@ func (ds *DistSender) divideAndSendBatchToRanges(
scanDir = Descending
seekKey = rs.EndKey
}
ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
ri.Seek(ctx, seekKey, scanDir)
if !ri.Valid() {
return nil, roachpb.NewError(ri.Error())
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (ds *DistSender) divideAndSendRangeFeedToRanges(
// boundaries. So, as we go, keep track of the remaining uncovered part of
// `rs` in `nextRS`.
nextRS := rs
ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
for ri.Seek(ctx, nextRS.Key, Ascending); ri.Valid(); ri.Next(ctx) {
desc := ri.Desc()
partialRS, err := nextRS.Intersect(desc)
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvclient/kvcoord/range_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ type RangeIterator struct {
err error
}

// NewRangeIterator creates a new RangeIterator.
func NewRangeIterator(ds *DistSender) *RangeIterator {
return &RangeIterator{
// MakeRangeIterator creates a new RangeIterator.
func MakeRangeIterator(ds *DistSender) RangeIterator {
return RangeIterator{
ds: ds,
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvclient/kvcoord/range_iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestRangeIterForward(t *testing.T) {
Settings: cluster.MakeTestingClusterSettings(),
})

ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
i := 0
span := roachpb.RSpan{
Key: testMetaEndKey,
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestRangeIterSeekForward(t *testing.T) {
Settings: cluster.MakeTestingClusterSettings(),
})

ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
i := 0
for ri.Seek(ctx, testMetaEndKey, Ascending); ri.Valid(); {
if !reflect.DeepEqual(alphaRangeDescriptors[i], *ri.Desc()) {
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestRangeIterReverse(t *testing.T) {
Settings: cluster.MakeTestingClusterSettings(),
})

ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
i := len(alphaRangeDescriptors) - 1
span := roachpb.RSpan{
Key: testMetaEndKey,
Expand Down Expand Up @@ -180,7 +180,7 @@ func TestRangeIterSeekReverse(t *testing.T) {
Settings: cluster.MakeTestingClusterSettings(),
})

ri := NewRangeIterator(ds)
ri := MakeRangeIterator(ds)
i := len(alphaRangeDescriptors) - 1
for ri.Seek(ctx, roachpb.RKey([]byte{'z'}), Descending); ri.Valid(); {
if !reflect.DeepEqual(alphaRangeDescriptors[i], *ri.Desc()) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_pipeliner.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ func (f rangeIteratorFactory) newRangeIterator() condensableSpanSetRangeIterator
return f.factory()
}
if f.ds != nil {
return NewRangeIterator(f.ds)
ri := MakeRangeIterator(f.ds)
return &ri
}
panic("no iterator factory configured")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/rangefeed/db_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (dbc *dbAdapter) divideAndSendScanRequests(

currentScanLimit := parallelismFn()
exportLim := limit.MakeConcurrentRequestLimiter("rangefeedScanLimiter", parallelismFn())
ri := kvcoord.NewRangeIterator(dbc.distSender)
ri := kvcoord.MakeRangeIterator(dbc.distSender)

for _, sp := range sg.Slice() {
nextRS, err := keys.SpanAddr(sp)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/gcjob/table_garbage_collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func clearSpanData(

var n int
lastKey := span.Key
ri := kvcoord.NewRangeIterator(distSender)
ri := kvcoord.MakeRangeIterator(distSender)
timer := timeutil.NewTimer()
defer timer.Stop()

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/physicalplan/span_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ type spanResolverIterator struct {
// txn is the transaction using the iterator.
txn *kv.Txn
// it is a wrapped RangeIterator.
it *kvcoord.RangeIterator
it kvcoord.RangeIterator
// oracle is used to choose a lease holders for ranges when one isn't present
// in the cache.
oracle replicaoracle.Oracle
Expand All @@ -170,7 +170,7 @@ var _ SpanResolverIterator = &spanResolverIterator{}
func (sr *spanResolver) NewSpanResolverIterator(txn *kv.Txn) SpanResolverIterator {
return &spanResolverIterator{
txn: txn,
it: kvcoord.NewRangeIterator(sr.distSender),
it: kvcoord.MakeRangeIterator(sr.distSender),
oracle: sr.oracle,
queryState: replicaoracle.MakeQueryState(),
}
Expand Down

0 comments on commit d54e0dd

Please sign in to comment.