[GLUTEN-10892][VL] Use `veloxPreferredBatchBytes` to control the max size of memory of batches combined by jiangjiangtian · Pull Request #11140 · apache/gluten

jiangjiangtian · 2025-11-21T03:27:07Z

What changes are proposed in this pull request?

Now the operator VeloxBatchResizer can use as many memory as possible, which can cause OOM. So this PR use the existing config spark.gluten.sql.columnar.backend.velox.preferredBatchBytes to control it.
Closes #10892

How was this patch tested?

Inner ETL jobs.

wForget · 2025-11-21T05:56:19Z

Could you add a new configuration for memoryThreshold and allow disabling it? I'm concerned that estimating batch size might cause performance overhead.

jiangjiangtian · 2025-11-21T06:31:14Z

Could you add a new configuration for memoryThreshold and allow disabling it? I'm concerned that estimating batch size might cause performance overhead.

Sure. Thanks for your advice.
Actually, I have tested 300 jobs and the overall execution time decrease by about 1.1%.

jinchengchenghh · 2025-11-27T10:35:05Z

@@ -680,6 +682,15 @@ object VeloxConfig extends ConfigRegistry {
      .bytesConf(ByteUnit.BYTE)
      .createWithDefaultString("10MB")

+  val ENABLE_LIMIT_BATCH_RESIZER = {


This can be a default behavior, please remove this config, all the operator should respect preferredBatchBytes

jinchengchenghh · 2025-11-27T10:36:46Z

@@ -60,7 +60,7 @@ Arguments: false

 (7) VeloxResizeBatches
 Input [18]: [hash_partition_key#X, l_returnflag#X, l_linestatus#X, sum#X, isEmpty#X, sum#X, isEmpty#X, sum#X, isEmpty#X, sum#X, isEmpty#X, sum#X, count#X, sum#X, count#X, sum#X, count#X, count#X]
-Arguments: X, X
+Arguments: X, X, 10485760


Please change it to X, we don't need to update it if we change the default value in the future

jinchengchenghh · 2025-11-27T10:38:09Z

    plan.transformUp {
      case shuffle: ColumnarShuffleExchangeExec
          if resizeBatchesShuffleInputEnabled &&
            shuffle.shuffleWriterType.requiresResizingShuffleInput =>
        val appendBatches =
-          VeloxResizeBatchesExec(shuffle.child, range.min, range.max)
+          VeloxResizeBatchesExec(shuffle.child, range.min, range.max, memoryThreshold)


Please use the original config name preferredBatchBytes instead of a new name

jinchengchenghh · 2025-11-27T10:43:15Z

@@ -78,22 +80,43 @@ std::shared_ptr<ColumnarBatch> VeloxBatchResizer::next() {
  if (cb->numRows() < minOutputBatchSize_) {


Please use cb->numBytes() to estimate the batch size, and update the numBytes(), get the numBytes does not need to flat the vector, just load the vector

This reverts commit 18bb790f472970a72af30a7db4f221c03779d25c.

jinchengchenghh · 2025-12-01T10:08:18Z

@@ -78,22 +80,41 @@ std::shared_ptr<ColumnarBatch> VeloxBatchResizer::next() {
  if (cb->numRows() < minOutputBatchSize_) {
    auto vb = VeloxColumnarBatch::from(pool_, cb);
    auto rv = vb->getRowVector();
+    auto vector = std::static_pointer_cast<facebook::velox::BaseVector>(rv);


I don't see the vector usage

jinchengchenghh · 2025-12-01T10:10:07Z

-      auto nextRv = nextVb->getRowVector();
-      if (buffer->size() + nextRv->size() > maxOutputBatchSize_) {
+    // Call reset manully to potentially release memory
+    vector.reset();


L114 is enough, you reuse the variables in following for loop or release in the end

And also remove following rv batch.reset

I remove the four lines of code here.

jinchengchenghh · 2025-12-01T10:11:26Z

@@ -78,22 +80,41 @@ std::shared_ptr<ColumnarBatch> VeloxBatchResizer::next() {
  if (cb->numRows() < minOutputBatchSize_) {
    auto vb = VeloxColumnarBatch::from(pool_, cb);
    auto rv = vb->getRowVector();
+    auto vector = std::static_pointer_cast<facebook::velox::BaseVector>(rv);
+    uint64_t numBytes = cb->numBytes();


Update here to only load the lazy vector, we don't need to flat dictionary vector https://github.com/apache/incubator-gluten/blob/7d491d5b373f77a3aa9a1bad14c34acb8987cfb5/cpp/velox/memory/VeloxColumnarBatch.cc#L82

I get it. Thanks!

jinchengchenghh · 2025-12-01T12:02:26Z

@@ -73,6 +73,7 @@ abstract class VeloxTPCHSuite extends VeloxTPCHTableSupport {
      // for unexpected blank
      .replaceAll("Scan parquet ", "Scan parquet")
      // Spark QueryStageExec will take it's id as argument, replace it with X
+      .replaceAll("Arguments: [0-9]+, [0-9]+, [0-9]+", "Arguments: X, X")


jinchengchenghh · 2025-12-01T12:03:54Z

-      auto nextRv = nextVb->getRowVector();
-      if (buffer->size() + nextRv->size() > maxOutputBatchSize_) {
+    // Call reset manully to potentially release memory
+    vector.reset();


And also remove following rv batch.reset

jinchengchenghh · 2025-12-01T12:14:05Z

@@ -78,22 +80,34 @@ std::shared_ptr<ColumnarBatch> VeloxBatchResizer::next() {
  if (cb->numRows() < minOutputBatchSize_) {
    auto vb = VeloxColumnarBatch::from(pool_, cb);
    auto rv = vb->getRowVector();
+    uint64_t numBytes = cb->numBytes();


Move the check with L80 if (cb->numRows() < minOutputBatchSize_ && numBytes <= preferredBatchBytes_)

Done. Thanks

jinchengchenghh · 2025-12-02T10:15:12Z

Thanks!

github-actions Bot added the VELOX label Nov 21, 2025

jiangjiangtian force-pushed the config_resize branch from 97d8201 to dbe19ba Compare November 21, 2025 03:46

jiangjiangtian force-pushed the config_resize branch from dbe19ba to 18bb790 Compare November 24, 2025 11:26

github-actions Bot added the DOCS label Nov 24, 2025

jiangjiangtian force-pushed the config_resize branch 3 times, most recently from 98e47b9 to 0e1128b Compare November 27, 2025 09:05

jinchengchenghh reviewed Nov 27, 2025

View reviewed changes

jiangtian added 8 commits December 1, 2025 17:47

add config to control the max size of memory of batches combined

9c03a41

update golden files

88d0bff

add config

68e0c2f

Revert "update golden files"

0befa08

This reverts commit 18bb790f472970a72af30a7db4f221c03779d25c.

update golden files

da7f7dd

fix

1cec509

update according to comment

fab47d1

update goldren files, change value to X

30fca1d

jiangjiangtian force-pushed the config_resize branch from 0e1128b to 30fca1d Compare December 1, 2025 09:48

github-actions Bot removed the DOCS label Dec 1, 2025

jinchengchenghh reviewed Dec 1, 2025

View reviewed changes

jiangjiangtian force-pushed the config_resize branch from 390136d to 0f04ea4 Compare December 1, 2025 12:10

jinchengchenghh reviewed Dec 1, 2025

View reviewed changes

jiangjiangtian force-pushed the config_resize branch from 0f04ea4 to 4299d0c Compare December 2, 2025 02:04

fix unit tests and resolve comments

532faa7

jiangjiangtian force-pushed the config_resize branch from 4299d0c to 532faa7 Compare December 2, 2025 02:51

jinchengchenghh approved these changes Dec 2, 2025

View reviewed changes

jinchengchenghh merged commit 7ac0217 into apache:main Dec 2, 2025
150 of 152 checks passed

		@@ -78,22 +80,43 @@ std::shared_ptr<ColumnarBatch> VeloxBatchResizer::next() {
		if (cb->numRows() < minOutputBatchSize_) {

Uh oh!

Conversation

jiangjiangtian commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

How was this patch tested?

Uh oh!

wForget commented Nov 21, 2025

Uh oh!

jiangjiangtian commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiangjiangtian Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinchengchenghh commented Dec 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jiangjiangtian commented Nov 21, 2025 •

edited

Loading

jiangjiangtian Dec 2, 2025 •

edited

Loading