Merge #95058 #95400

95058: rangefeed: Cleanup frontier usage r=miretskiy a=miretskiy Frontier is thread safe, so there is no need to maintain additional lock. Epic: None Release note: None 95400: coldataext: remove an allocation in production builds for arrays r=yuzefovich a=yuzefovich This commit makes it so that we only perform assertions in the datum vec in the test builds. In particular, this allows us to remove an allocation for each array object being set (which happens in `tree.DArray.ResolvedType` where we construct an array type on demand). I was just looking at a heap profile where this allocation accounted for 7% of all allocations. Epic: None Release note: None Co-authored-by: Yevgeniy Miretskiy <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
cockroachdb · Jan 17, 2023 · 4ec5a5f · 4ec5a5f
3 parents 15c4bff + dde642d + 990ef9c
commit 4ec5a5f
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 9 deletions.
diff --git a/pkg/col/coldataext/BUILD.bazel b/pkg/col/coldataext/BUILD.bazel
@@ -19,6 +19,7 @@ go_library(
         "//pkg/sql/sem/eval",
         "//pkg/sql/sem/tree",
         "//pkg/sql/types",
+        "//pkg/util/buildutil",
         "@com_github_cockroachdb_errors//:errors",
     ],
 )

diff --git a/pkg/col/coldataext/datum_vec.go b/pkg/col/coldataext/datum_vec.go
@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -90,7 +91,9 @@ func (dv *datumVec) Get(i int) coldata.Datum {
 // Set implements coldata.DatumVec interface.
 func (dv *datumVec) Set(i int, v coldata.Datum) {
 	datum := convertToDatum(v)
-	dv.assertValidDatum(datum)
+	if buildutil.CrdbTestBuild {
+		dv.assertValidDatum(datum)
+	}
 	dv.data[i] = datum
 }
 
@@ -106,21 +109,27 @@ func (dv *datumVec) Window(start, end int) coldata.DatumVec {
 // CopySlice implements coldata.DatumVec interface.
 func (dv *datumVec) CopySlice(src coldata.DatumVec, destIdx, srcStartIdx, srcEndIdx int) {
 	castSrc := src.(*datumVec)
-	dv.assertSameTypeFamily(castSrc.t)
+	if buildutil.CrdbTestBuild {
+		dv.assertSameTypeFamily(castSrc.t)
+	}
 	copy(dv.data[destIdx:], castSrc.data[srcStartIdx:srcEndIdx])
 }
 
 // AppendSlice implements coldata.DatumVec interface.
 func (dv *datumVec) AppendSlice(src coldata.DatumVec, destIdx, srcStartIdx, srcEndIdx int) {
 	castSrc := src.(*datumVec)
-	dv.assertSameTypeFamily(castSrc.t)
+	if buildutil.CrdbTestBuild {
+		dv.assertSameTypeFamily(castSrc.t)
+	}
 	dv.data = append(dv.data[:destIdx], castSrc.data[srcStartIdx:srcEndIdx]...)
 }
 
 // AppendVal implements coldata.DatumVec interface.
 func (dv *datumVec) AppendVal(v coldata.Datum) {
 	datum := convertToDatum(v)
-	dv.assertValidDatum(datum)
+	if buildutil.CrdbTestBuild {
+		dv.assertValidDatum(datum)
+	}
 	dv.data = append(dv.data, datum)
 }
 

diff --git a/pkg/kv/kvclient/rangefeed/scanner.go b/pkg/kv/kvclient/rangefeed/scanner.go
@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"github.com/cockroachdb/cockroach/pkg/util/span"
-	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 )
 
 // runInitialScan will attempt to perform an initial data scan.
@@ -102,16 +101,13 @@ func (f *RangeFeed) getSpansToScan(ctx context.Context) func() []roachpb.Span {
 		return retryAll
 	}
 
-	var fm syncutil.Mutex
 	userSpanDoneCallback := f.onSpanDone
 	f.onSpanDone = func(ctx context.Context, sp roachpb.Span) error {
 		if userSpanDoneCallback != nil {
 			if err := userSpanDoneCallback(ctx, sp); err != nil {
 				return err
 			}
 		}
-		fm.Lock()
-		defer fm.Unlock()
 		_, err := frontier.Forward(sp, f.initialTimestamp)
 		return err
 	}
@@ -133,5 +129,4 @@ func (f *RangeFeed) getSpansToScan(ctx context.Context) func() []roachpb.Span {
 		})
 		return retrySpans
 	}
-
 }