Skip to content

Commit

Permalink
kvcoord: optimize Truncate
Browse files Browse the repository at this point in the history
This commit optimizes the `Truncate` function by accessing the header of
the request directly (instead of having to construct `roachpb.Span`) as
well as keeping track of whether the request has been changed (instead
of performing key comparison later).
```
name                              old time/op    new time/op    delta
Truncate/reqs=32/type=get-24        3.81µs ± 4%    3.42µs ± 1%  -10.32%  (p=0.000 n=9+9)
Truncate/reqs=32/type=scan-24       6.35µs ±11%    5.81µs ±11%   -8.56%  (p=0.019 n=10+10)
Truncate/reqs=1024/type=get-24      98.9µs ± 2%    85.8µs ± 1%  -13.24%  (p=0.000 n=10+10)
Truncate/reqs=1024/type=scan-24      176µs ± 2%     155µs ± 2%  -12.17%  (p=0.000 n=10+10)
Truncate/reqs=32768/type=get-24     3.23ms ± 1%    2.78ms ± 0%  -13.86%  (p=0.000 n=10+9)
Truncate/reqs=32768/type=scan-24    5.64ms ± 1%    5.02ms ± 0%  -11.01%  (p=0.000 n=10+9)

name                              old alloc/op   new alloc/op   delta
Truncate/reqs=32/type=get-24          744B ± 0%      744B ± 0%     ~     (all equal)
Truncate/reqs=32/type=scan-24       2.08kB ±37%    2.05kB ±36%     ~     (p=0.897 n=10+10)
Truncate/reqs=1024/type=get-24      24.6kB ± 0%    24.6kB ± 0%     ~     (all equal)
Truncate/reqs=1024/type=scan-24     73.2kB ± 2%    73.7kB ± 2%     ~     (p=0.167 n=9+9)
Truncate/reqs=32768/type=get-24     1.21MB ± 0%    1.21MB ± 0%     ~     (p=0.370 n=10+10)
Truncate/reqs=32768/type=scan-24    2.82MB ± 0%    2.84MB ± 0%   +0.48%  (p=0.000 n=10+10)

name                              old allocs/op  new allocs/op  delta
Truncate/reqs=32/type=get-24          10.0 ± 0%      10.0 ± 0%     ~     (all equal)
Truncate/reqs=32/type=scan-24         33.6 ±25%      34.8 ±26%     ~     (p=0.646 n=10+10)
Truncate/reqs=1024/type=get-24        20.0 ± 0%      20.0 ± 0%     ~     (all equal)
Truncate/reqs=1024/type=scan-24        692 ± 5%       705 ± 6%     ~     (p=0.167 n=9+9)
Truncate/reqs=32768/type=get-24       40.0 ± 0%      40.0 ± 0%     ~     (all equal)
Truncate/reqs=32768/type=scan-24     21.6k ± 2%     22.0k ± 1%   +1.73%  (p=0.000 n=10+10)
```

Release note: None
  • Loading branch information
yuzefovich committed Jun 6, 2022
1 parent 63e7c5b commit 689788c
Showing 1 changed file with 21 additions and 21 deletions.
42 changes: 21 additions & 21 deletions pkg/kv/kvclient/kvcoord/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/cockroachdb/errors"
)

var emptySpan = roachpb.Span{}
var emptyHeader = roachpb.RequestHeader{}

// Truncate restricts all requests to the given key range and returns new,
// truncated, requests. All returned requests are "truncated" to the given span,
Expand All @@ -31,41 +31,42 @@ var emptySpan = roachpb.Span{}
func Truncate(
reqs []roachpb.RequestUnion, rs roachpb.RSpan,
) ([]roachpb.RequestUnion, []int, error) {
truncateOne := func(args roachpb.Request) (bool, roachpb.Span, error) {
header := args.Header().Span()
truncateOne := func(args roachpb.Request) (hasRequest bool, changed bool, _ roachpb.RequestHeader, _ error) {
header := args.Header()
if !roachpb.IsRange(args) {
// This is a point request.
if len(header.EndKey) > 0 {
return false, emptySpan, errors.Errorf("%T is not a range command, but EndKey is set", args)
return false, false, emptyHeader, errors.Errorf("%T is not a range command, but EndKey is set", args)
}
keyAddr, err := keys.Addr(header.Key)
if err != nil {
return false, emptySpan, err
return false, false, emptyHeader, err
}
if !rs.ContainsKey(keyAddr) {
return false, emptySpan, nil
return false, false, emptyHeader, nil
}
return true, header, nil
return true, false, header, nil
}
// We're dealing with a range-spanning request.
local := false
keyAddr, err := keys.Addr(header.Key)
if err != nil {
return false, emptySpan, err
return false, false, emptyHeader, err
}
endKeyAddr, err := keys.Addr(header.EndKey)
if err != nil {
return false, emptySpan, err
return false, false, emptyHeader, err
}
if l, r := keys.IsLocal(header.Key), keys.IsLocal(header.EndKey); l || r {
if !l || !r {
return false, emptySpan, errors.Errorf("local key mixed with global key in range")
return false, false, emptyHeader, errors.Errorf("local key mixed with global key in range")
}
local = true
}
if keyAddr.Less(rs.Key) {
// rs.Key can't be local because it contains range split points, which
// are never local.
changed = true
if !local {
header.Key = rs.Key.AsRawKey()
} else {
Expand All @@ -77,6 +78,7 @@ func Truncate(
if !endKeyAddr.Less(rs.EndKey) {
// rs.EndKey can't be local because it contains range split points, which
// are never local.
changed = true
if !local {
header.EndKey = rs.EndKey.AsRawKey()
} else {
Expand All @@ -87,10 +89,10 @@ func Truncate(
}
// Check whether the truncation has left any keys in the range. If not,
// we need to cut it out of the request.
if header.Key.Compare(header.EndKey) >= 0 {
return false, emptySpan, nil
if changed && header.Key.Compare(header.EndKey) >= 0 {
return false, false, emptyHeader, nil
}
return true, header, nil
return true, changed, header, nil
}

// TODO(tschottdorf): optimize so that we don't always make a new request
Expand All @@ -99,19 +101,17 @@ func Truncate(
var positions []int
var truncReqs []roachpb.RequestUnion
for pos, arg := range reqs {
hasRequest, newSpan, err := truncateOne(arg.GetInner())
inner := arg.GetInner()
hasRequest, changed, newHeader, err := truncateOne(inner)
if hasRequest {
// Keep the old one. If we must adjust the header, must copy.
inner := reqs[pos].GetInner()
oldHeader := inner.Header()
if newSpan.EqualValue(oldHeader.Span()) {
truncReqs = append(truncReqs, reqs[pos])
} else {
oldHeader.SetSpan(newSpan)
if changed {
shallowCopy := inner.ShallowCopy()
shallowCopy.SetHeader(oldHeader)
shallowCopy.SetHeader(newHeader)
truncReqs = append(truncReqs, roachpb.RequestUnion{})
truncReqs[len(truncReqs)-1].MustSetInner(shallowCopy)
} else {
truncReqs = append(truncReqs, arg)
}
positions = append(positions, pos)
}
Expand Down

0 comments on commit 689788c

Please sign in to comment.