zhli1142015 commented on code in PR #12101: URL: https://github.com/apache/gluten/pull/12101#discussion_r3265402305
########## cpp/velox/benchmarks/VeloxBatchResizerBenchmark.cc: ########## @@ -0,0 +1,568 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <limits> +#include <memory> +#include <optional> +#include <string> +#include <vector> + +#include <arrow/util/bit_util.h> +#include <arrow/util/bitmap_ops.h> +#include <benchmark/benchmark.h> + +#include "memory/ColumnarBatchIterator.h" +#include "memory/VeloxColumnarBatch.h" +#include "shuffle/Payload.h" +#include "utils/Exception.h" +#include "utils/VeloxBatchResizer.h" +#include "velox/common/memory/Memory.h" +#include "velox/vector/ComplexVector.h" +#include "velox/vector/FlatVector.h" + +using namespace facebook::velox; + +namespace gluten { +namespace { + +constexpr int32_t kInputBatches = 64; +constexpr int32_t kRowsPerBatch = 64; +constexpr int32_t kTotalRows = kInputBatches * kRowsPerBatch; +constexpr int64_t kPreferredBatchBytes = std::numeric_limits<int64_t>::max(); Review Comment: I added two fallback-focused benchmark scenarios for the copyRanges default-on path: - `DictionaryHeavy_64x64` - `ConstantHeavy_64x64` Each scenario compares: 1. `BM_VeloxBatchResizerFallbackAppendOptOutBaseline`: `enableCopyRanges=false`, the old incremental append path. 2. `BM_VeloxBatchResizerDefaultCopyRangesFallback`: default `enableCopyRanges=true`; dictionary/constant inputs are unsupported by child `copyRanges`, so they fall back to `RowVector::copy` inside the new collect-and-copy path. Benchmark command: ```bash /tmp/gluten-cpp-ut-build/velox/benchmarks/velox_batch_resizer_benchmark \ --benchmark_min_time=0.05s \ --benchmark_repetitions=3 \ --benchmark_format=json \ > /tmp/velox_batch_resizer_perf.json Environment: - CPU: 32 logical CPUs @ ~2995 MHz - Caches: L1D 48 KiB x16, L1I 32 KiB x16, L2 2 MiB x16, L3 36 MiB x1 - Note: ASLR was enabled, so the numbers may contain normal benchmark noise. Dense flat scenarios, CPU time averaged over 3 repetitions: ┌──────────────────┬─────────────────────────┬────────────────────┬─────────┬─────────────────────────┬─────────────────────────────┐ │ Scenario │ Append opt-out baseline │ Default copyRanges │ Speedup │ Direct child copyRanges │ Raw payload bulk-copy model │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ Mixed_64x64 │ 94.3 us │ 11.3 us │ 8.35x │ 14.4 us │ 34.8 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ Mixed_16x256 │ 31.7 us │ 6.3 us │ 5.01x │ 5.3 us │ 11.2 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ Mixed_256x16 │ 234.2 us │ 48.7 us │ 4.81x │ 30.6 us │ 119.2 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ Fixed2_64x64 │ 29.3 us │ 5.4 us │ 5.43x │ 2.1 us │ 13.4 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ Fixed16_64x64 │ 123.5 us │ 26.7 us │ 4.62x │ 18.6 us │ 95.7 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ LongString_64x64 │ 33.3 us │ 7.2 us │ 4.61x │ 4.1 us │ 15.7 us │ ├──────────────────┼─────────────────────────┼────────────────────┼─────────┼─────────────────────────┼─────────────────────────────┤ │ BoolHeavy_64x64 │ 73.4 us │ 10.8 us │ 6.78x │ 5.4 us │ 38.1 us │ └──────────────────┴─────────────────────────┴────────────────────┴─────────┴─────────────────────────┴─────────────────────────────┘ Fallback scenarios requested by the review comment, CPU time averaged over 3 repetitions: ┌───────────────────────┬─────────────────────────┬─────────────────────────────┬──────────────┐ │ Scenario │ Append opt-out baseline │ Default copyRanges fallback │ Result │ ├───────────────────────┼─────────────────────────┼─────────────────────────────┼──────────────┤ │ DictionaryHeavy_64x64 │ 102.7 us │ 77.7 us │ 1.32x faster │ ├───────────────────────┼─────────────────────────┼─────────────────────────────┼──────────────┤ │ ConstantHeavy_64x64 │ 67.8 us │ 42.0 us │ 1.61x faster │ └───────────────────────┴─────────────────────────┴─────────────────────────────┴──────────────┘ The fallback numbers are not regressions. The reason is that enableCopyRanges=true fallback is not the same as the old enableCopyRanges=false append path. With enableCopyRanges=false, VeloxBatchResizer creates an empty output RowVector and incrementally appends each small input batch via buffer->append(input). That path grows and updates the output vector batch-by-batch. With enableCopyRanges=true, even when a batch is not eligible for child-vector copyRanges because it contains dictionary or constant children, the resizer first collects the small input batches, computes the final totalRows, allocates/resizes the output RowVector once, and then copies each unsupported input into the pre-sized output using: buffer->copy(input.get(), offset, 0, input->size()); So these fallback benchmarks still benefit from the new collect-and-copy structure: - one output allocation / resize for the final row count; - fixed-offset RowVector::copy into a pre-sized output; - less incremental append bookkeeping; - Velox copy still has encoded-vector handling for dictionary and constant inputs, even though child copyRanges is not used. This explains why the default copyRanges-enabled fallback path is faster than the append opt-out baseline in these two scenarios. The important validation point for the review is that dictionary-heavy and constant-heavy inputs do not show a perf regression after enabling copyRanges by default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
