Skip to content

Commit 888140d

Browse files
committed
kvcoord: eliminate bounds checks in truncate loops
Release note: None
1 parent 8533536 commit 888140d

File tree

1 file changed

+90
-46
lines changed

1 file changed

+90
-46
lines changed

pkg/kv/kvclient/kvcoord/batch.go

Lines changed: 90 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -476,50 +476,69 @@ func (h *BatchTruncationHelper) Truncate(
476476
// non-trivial slowdown and increase in allocations, so we choose to duplicate
477477
// the code for performance.
478478
func (h *BatchTruncationHelper) truncateAsc(rs roachpb.RSpan) ([]kvpb.RequestUnion, []int, error) {
479-
endIdx := len(h.positions) - 1
479+
endIdx := len(h.positions)
480+
headers := h.headers[h.startIdx:endIdx]
481+
positions := h.positions[h.startIdx:endIdx]
482+
requests := h.requests[h.startIdx:endIdx]
483+
isRange := h.isRange[h.startIdx:endIdx]
484+
endIdx = len(positions)
485+
480486
fullyProcessed := 0
481-
for i := h.startIdx; i < len(h.positions); i++ {
482-
pos := h.positions[i]
487+
for i := range positions {
488+
//gcassert:bce
489+
pos := positions[i]
483490
if pos < 0 {
484491
fullyProcessed++
485492
continue
486493
}
487-
header := h.headers[i]
494+
//gcassert:bce
495+
header := headers[i]
488496
ek := rs.EndKey.AsRawKey()
489497
if ek.Compare(header.Key) <= 0 {
490498
// All of the remaining requests start after this range, so we're
491499
// done.
492-
endIdx = i - 1
500+
endIdx = i
493501
break
494502
}
495503
}
496504

497-
numReqs := endIdx - h.startIdx - fullyProcessed + 1
505+
headers = headers[:endIdx]
506+
positions = positions[:endIdx]
507+
requests = requests[:endIdx]
508+
isRange = isRange[:endIdx]
509+
numReqs := len(positions) - fullyProcessed
498510
if numReqs == 0 {
499511
return nil, nil, nil
500512
}
501513
truncReqs := make([]kvpb.RequestUnion, 0, numReqs)
502-
positions := make([]int, 0, numReqs)
514+
truncPositions := make([]int, 0, numReqs)
503515

504-
for i := h.startIdx; i <= endIdx; i++ {
505-
pos := h.positions[i]
516+
for i := range positions {
517+
//gcassert:bce
518+
pos := positions[i]
506519
if pos < 0 {
507520
// This request has already been fully processed, so there is no
508521
// need to look at it.
509522
continue
510523
}
511-
header := h.headers[i]
524+
//gcassert:bce
525+
header := headers[i]
512526
// rs.EndKey can't be local because it contains range split points,
513527
// which are never local.
514528
ek := rs.EndKey.AsRawKey()
515-
if !h.isRange[i] {
529+
//gcassert:bce
530+
req := requests[i]
531+
//gcassert:bce
532+
if !isRange[i] {
516533
// This is a point request, and the key is contained within this
517534
// range, so we include the request as is and mark it as "fully
518535
// processed".
519-
truncReqs = append(truncReqs, h.requests[i])
520-
positions = append(positions, pos)
521-
h.headers[i] = kvpb.RequestHeader{}
522-
h.positions[i] = -1
536+
truncReqs = append(truncReqs, req)
537+
truncPositions = append(truncPositions, pos)
538+
//gcassert:bce
539+
headers[i] = kvpb.RequestHeader{}
540+
//gcassert:bce
541+
positions[i] = -1
523542
continue
524543
}
525544
// We're dealing with a range-spanning request.
@@ -532,32 +551,35 @@ func (h *BatchTruncationHelper) truncateAsc(rs roachpb.RSpan) ([]kvpb.RequestUni
532551
)
533552
}
534553
}
535-
inner := h.requests[i].GetInner()
554+
inner := req.GetInner()
536555
if header.EndKey.Compare(ek) <= 0 {
537556
// This is the last part of this request since it is fully contained
538557
// within this range, so we mark the request as "fully processed".
539-
h.headers[i] = kvpb.RequestHeader{}
540-
h.positions[i] = -1
558+
//gcassert:bce
559+
headers[i] = kvpb.RequestHeader{}
560+
//gcassert:bce
561+
positions[i] = -1
541562
if origStartKey := inner.Header().Key; origStartKey.Equal(header.Key) {
542563
// This range-spanning request fits within a single range, so we
543564
// can just use the original request.
544-
truncReqs = append(truncReqs, h.requests[i])
545-
positions = append(positions, pos)
565+
truncReqs = append(truncReqs, req)
566+
truncPositions = append(truncPositions, pos)
546567
continue
547568
}
548569
} else {
549570
header.EndKey = ek
550571
// Adjust the start key of the header so that it contained only the
551572
// unprocessed suffix of the request.
552-
h.headers[i].Key = header.EndKey
573+
//gcassert:bce
574+
headers[i].Key = header.EndKey
553575
}
554576
shallowCopy := inner.ShallowCopy()
555577
shallowCopy.SetHeader(header)
556578
truncReqs = append(truncReqs, kvpb.RequestUnion{})
557579
truncReqs[len(truncReqs)-1].MustSetInner(shallowCopy)
558-
positions = append(positions, pos)
580+
truncPositions = append(truncPositions, pos)
559581
}
560-
return truncReqs, positions, nil
582+
return truncReqs, truncPositions, nil
561583
}
562584

563585
// truncateDesc is the optimized strategy for Truncate() with the Descending
@@ -653,50 +675,69 @@ func (h *BatchTruncationHelper) truncateAsc(rs roachpb.RSpan) ([]kvpb.RequestUni
653675
// non-trivial slowdown and increase in allocations, so we choose to duplicate
654676
// the code for performance.
655677
func (h *BatchTruncationHelper) truncateDesc(rs roachpb.RSpan) ([]kvpb.RequestUnion, []int, error) {
656-
endIdx := len(h.positions) - 1
678+
endIdx := len(h.positions)
679+
headers := h.headers[h.startIdx:endIdx]
680+
positions := h.positions[h.startIdx:endIdx]
681+
requests := h.requests[h.startIdx:endIdx]
682+
isRange := h.isRange[h.startIdx:endIdx]
683+
endIdx = len(positions)
684+
657685
fullyProcessed := 0
658-
for i := h.startIdx; i < len(h.positions); i++ {
659-
pos := h.positions[i]
686+
for i := range positions {
687+
//gcassert:bce
688+
pos := positions[i]
660689
if pos < 0 {
661690
fullyProcessed++
662691
continue
663692
}
664-
header := h.headers[i]
693+
//gcassert:bce
694+
header := headers[i]
665695
sk := rs.Key.AsRawKey()
666696
if sk.Compare(header.EndKey) >= 0 {
667697
// All of the remaining requests end before this range, so we're
668698
// done.
669-
endIdx = i - 1
699+
endIdx = i
670700
break
671701
}
672702
}
673703

674-
numReqs := endIdx - h.startIdx - fullyProcessed + 1
704+
headers = headers[:endIdx]
705+
positions = positions[:endIdx]
706+
requests = requests[:endIdx]
707+
isRange = isRange[:endIdx]
708+
numReqs := len(positions) - fullyProcessed
675709
if numReqs == 0 {
676710
return nil, nil, nil
677711
}
678712
truncReqs := make([]kvpb.RequestUnion, 0, numReqs)
679-
positions := make([]int, 0, numReqs)
713+
truncPositions := make([]int, 0, numReqs)
680714

681-
for i := h.startIdx; i <= endIdx; i++ {
682-
pos := h.positions[i]
715+
for i := range positions {
716+
//gcassert:bce
717+
pos := positions[i]
683718
if pos < 0 {
684719
// This request has already been fully processed, so there is no
685720
// need to look at it.
686721
continue
687722
}
688-
header := h.headers[i]
723+
//gcassert:bce
724+
header := headers[i]
689725
// rs.Key can't be local because it contains range split points, which
690726
// are never local.
691727
sk := rs.Key.AsRawKey()
692-
if !h.isRange[i] {
728+
//gcassert:bce
729+
req := requests[i]
730+
//gcassert:bce
731+
if !isRange[i] {
693732
// This is a point request, and the key is contained within this
694733
// range, so we include the request as is and mark it as "fully
695734
// processed".
696-
truncReqs = append(truncReqs, h.requests[i])
697-
positions = append(positions, pos)
698-
h.headers[i] = kvpb.RequestHeader{}
699-
h.positions[i] = -1
735+
truncReqs = append(truncReqs, req)
736+
truncPositions = append(truncPositions, pos)
737+
//gcassert:bce
738+
headers[i] = kvpb.RequestHeader{}
739+
//gcassert:bce
740+
positions[i] = -1
700741
continue
701742
}
702743
// We're dealing with a range-spanning request.
@@ -709,32 +750,35 @@ func (h *BatchTruncationHelper) truncateDesc(rs roachpb.RSpan) ([]kvpb.RequestUn
709750
)
710751
}
711752
}
712-
inner := h.requests[i].GetInner()
753+
inner := req.GetInner()
713754
if header.Key.Compare(sk) >= 0 {
714755
// This is the last part of this request since it is fully contained
715756
// within this range, so we mark the request as "fully processed".
716-
h.headers[i] = kvpb.RequestHeader{}
717-
h.positions[i] = -1
757+
//gcassert:bce
758+
headers[i] = kvpb.RequestHeader{}
759+
//gcassert:bce
760+
positions[i] = -1
718761
if origEndKey := inner.Header().EndKey; len(origEndKey) == 0 || origEndKey.Equal(header.EndKey) {
719762
// This range-spanning request fits within a single range, so we
720763
// can just use the original request.
721-
truncReqs = append(truncReqs, h.requests[i])
722-
positions = append(positions, pos)
764+
truncReqs = append(truncReqs, req)
765+
truncPositions = append(truncPositions, pos)
723766
continue
724767
}
725768
} else {
726769
header.Key = sk
727770
// Adjust the end key of the header so that it contained only the
728771
// unprocessed prefix of the request.
729-
h.headers[i].EndKey = header.Key
772+
//gcassert:bce
773+
headers[i].EndKey = header.Key
730774
}
731775
shallowCopy := inner.ShallowCopy()
732776
shallowCopy.SetHeader(header)
733777
truncReqs = append(truncReqs, kvpb.RequestUnion{})
734778
truncReqs[len(truncReqs)-1].MustSetInner(shallowCopy)
735-
positions = append(positions, pos)
779+
truncPositions = append(truncPositions, pos)
736780
}
737-
return truncReqs, positions, nil
781+
return truncReqs, truncPositions, nil
738782
}
739783

740784
var emptyHeader = kvpb.RequestHeader{}

0 commit comments

Comments
 (0)