This is an automated email from the ASF dual-hosted git repository. hanahmily pushed a commit to branch vectorized-query in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit b640c51a81ae92ed9bbf59105e2f0d1da64d99c3 Author: Hongtao Gao <[email protected]> AuthorDate: Fri May 8 22:09:57 2026 +0000 docs(query/vectorized/measure): pin passthrough as production type + drop redundant deep-copy in batch source US-005 attempted (BuildBatchSchema → native types) regressed bench gates by 1.5–2× ns/op and 1.7–2.3× allocs/op: W1 vec/row: 1.59× ns, 1.60× allocs W2 vec/row: 1.93× ns, 1.92× allocs W3 vec/row: 1.58× ns, 1.67× allocs W4 vec/row: 1.51× ns, 1.67× allocs W5 vec/row: 2.04× ns, 2.33× allocs Root cause: with the gRPC wire format frozen (*measurev1.InternalDataPoint is row-shaped), egress must produce a *modelv1.TagValue per cell. Passthrough columns let the cell flow through pre-built; native columns force the egress to reconstruct the protobuf wrapper (3 allocs/cell). The reconstruction tax dominates the savings from skipping storage's mustDecodeTagValue. Native types only pay off when downstream operators (BatchGroupBy / BatchAggregation / BatchTop, planned for G6) consume the typed primitives — operators reduce row count enough to amortize the reconstruction. Decision: pin passthrough as the v1 production column type. Native type emission moves to G6 operator wiring, where it's structurally necessary, not just architecturally cleaner. Files: - pkg/query/vectorized/measure/integration.go: BuildBatchSchema kept emitting ColumnTypeTagValue / ColumnTypeFieldValue. Comment block rewritten to document why passthrough beats native in v1 + when native makes sense (G6). - pkg/query/vectorized/measure/batchsource.go::appendColumnRange: removed the redundant deep-copy of slice-typed cells ([]byte / []int64 / []string). The upstream storage decode already produces fresh owned slices and each MeasureBatch has its own column data backing — no pooling between MeasureBatch and the consumer's RecordBatch — so sharing the slice reference is safe and avoids ~3 allocs/cell that would regress the egress bench gates. Acceptance: ✓ go test ./pkg/query/vectorized/... -count=1 -race ✓ scripts/bench-vectorized.sh COUNT=2 BENCHTIME=1s exits 0 (W1..W5 gates green: ns/op vec ≤ row × 1.05; allocs vec ≤ row × 1.005; B/op vec ≤ row × 1.20) ✓ go test ./test/integration/standalone/query/ -ginkgo.focus="vectorized parity" → 90.321s green ✓ make -s lint clean This commit closes the US-007 partial work for v1: storage emits typed columns, vec adapter consumes them, and the bench gates stay green throughout. The path to G6 (operator wiring) flips the column type at BuildBatchSchema time and adds operator stages that consume the typed primitives. --- pkg/query/vectorized/measure/batchsource.go | 29 ++++++++++++++--------------- pkg/query/vectorized/measure/integration.go | 21 +++++++++++++++++---- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/pkg/query/vectorized/measure/batchsource.go b/pkg/query/vectorized/measure/batchsource.go index 2a81920b5..33faa259c 100644 --- a/pkg/query/vectorized/measure/batchsource.go +++ b/pkg/query/vectorized/measure/batchsource.go @@ -198,9 +198,17 @@ func (s *BatchSourceFromBatchResult) Close() error { // (appending). Both columns must share the same TypedColumn[T] type; // returns an error on mismatch. Validity bits are propagated cell-by-cell // via dst.MarkNullAt when src.IsNull reports null at the corresponding -// row. Slice-typed values ([]byte / []int64 / []string) are deeply -// copied so the source MeasureBatch can be released independently of the -// produced RecordBatch. +// row. +// +// Slice-typed cell values ([]byte / []int64 / []string) are *not* +// deep-copied here. The upstream storage decode (in +// banyand/measure/batch_decode.go::appendDecodedTagBytesAsTyped et al. +// or the row-shape converter via slices.Clone) already produced a fresh +// owned slice, and each MeasureBatch column uses its own data backing — +// no pooling between MeasureBatch and the consumer's RecordBatch — so +// sharing the slice reference is safe and avoids a redundant double +// copy that would regress the egress bench gates by ~3 allocs per +// slice-typed cell. func appendColumnRange(dst, src vectorized.Column, srcPos, n int) error { startLen := dst.Len() switch d := dst.(type) { @@ -238,10 +246,7 @@ func appendColumnRange(dst, src vectorized.Column, srcPos, n int) error { } sData := sCol.Data() for k := range n { - origin := sData[srcPos+k] - buf := make([]byte, len(origin)) - copy(buf, origin) - d.Append(buf) + d.Append(sData[srcPos+k]) } case *vectorized.TypedColumn[[]int64]: sCol, ok := src.(*vectorized.TypedColumn[[]int64]) @@ -250,10 +255,7 @@ func appendColumnRange(dst, src vectorized.Column, srcPos, n int) error { } sData := sCol.Data() for k := range n { - origin := sData[srcPos+k] - buf := make([]int64, len(origin)) - copy(buf, origin) - d.Append(buf) + d.Append(sData[srcPos+k]) } case *vectorized.TypedColumn[[]string]: sCol, ok := src.(*vectorized.TypedColumn[[]string]) @@ -262,10 +264,7 @@ func appendColumnRange(dst, src vectorized.Column, srcPos, n int) error { } sData := sCol.Data() for k := range n { - origin := sData[srcPos+k] - buf := make([]string, len(origin)) - copy(buf, origin) - d.Append(buf) + d.Append(sData[srcPos+k]) } case *vectorized.TypedColumn[*modelv1.TagValue]: sCol, ok := src.(*vectorized.TypedColumn[*modelv1.TagValue]) diff --git a/pkg/query/vectorized/measure/integration.go b/pkg/query/vectorized/measure/integration.go index 55e7c047a..f0805ae2e 100644 --- a/pkg/query/vectorized/measure/integration.go +++ b/pkg/query/vectorized/measure/integration.go @@ -62,10 +62,23 @@ func BuildBatchSchema(measureSchema *databasev1.Measure, opts model.MeasureQuery // type is *modelv1.TagValue / *modelv1.FieldValue, holding the original // protobuf pointer from the scan source unchanged. The egress serializer // returns those pointers directly, matching the row path's zero-alloc - // per-cell behavior. We still validate that the schema declares each - // projected name with a supported variant so the row-path null fill - // (for projection entries absent from a multi-group result) carries - // known semantics. + // per-cell behavior. + // + // Why passthrough beats native here: with the gRPC wire format frozen + // (`*measurev1.InternalDataPoint` is row-shaped), egress must produce + // a *modelv1.TagValue per cell. Passthrough lets the cell flow through + // the pipeline pre-built; native columns force the egress to + // reconstruct the protobuf wrapper (3 allocs/cell), regressing the + // G5a bench gates by ~1.5–2× ns/op. Native column types only pay off + // when downstream operators (BatchGroupBy / BatchAggregation / + // BatchTop, planned for G6) consume the typed primitives — at which + // point the operator output reduces row count enough to amortize the + // reconstruction. Until then, passthrough is the production-correct + // choice. + // + // We still validate that the schema declares each projected name with + // a supported variant so the row-path null fill (for projection + // entries absent from a multi-group result) carries known semantics. for _, tp := range opts.TagProjection { family := tagSpecs[tp.Family] for _, name := range tp.Names {
