From dde642de05da5e0ba64861f290e490030c4caeb5 Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Wed, 11 Jan 2023 08:43:57 -0500 Subject: [PATCH 1/2] rangefeed: Cleanup frontier usage Frontier is thread safe, so there is no need to maintain additional lock. Epic: None Release note: None --- pkg/kv/kvclient/rangefeed/scanner.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/kv/kvclient/rangefeed/scanner.go b/pkg/kv/kvclient/rangefeed/scanner.go index 4390633befa1..93e87cc87ff2 100644 --- 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,7 +101,6 @@ 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 { @@ -110,8 +108,6 @@ func (f *RangeFeed) getSpansToScan(ctx context.Context) func() []roachpb.Span { 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 } - } From 990ef9c4bd9a421b2576e9f6f980700999a55b0c Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Tue, 17 Jan 2023 14:16:20 -0800 Subject: [PATCH 2/2] coldataext: remove an allocation in production builds for arrays 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. Release note: None --- pkg/col/coldataext/BUILD.bazel | 1 + pkg/col/coldataext/datum_vec.go | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/col/coldataext/BUILD.bazel b/pkg/col/coldataext/BUILD.bazel index d4910afff48b..5f258b8e9882 100644 --- 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 index 884640ab7f09..7ad5ccb3e1c9 100644 --- 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) }