Skip to content

[VL] Materialize CudfVector to CPU columns before host code reads them#12416

Open
ReemaAlzaid wants to merge 2 commits into
apache:mainfrom
ReemaAlzaid:gpu-cudf-materialize-cudfvector
Open

[VL] Materialize CudfVector to CPU columns before host code reads them#12416
ReemaAlzaid wants to merge 2 commits into
apache:mainfrom
ReemaAlzaid:gpu-cudf-materialize-cudfvector

Conversation

@ReemaAlzaid

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

With the Velox cuDF (GPU) backend, GPU results are wrapped in CudfVector a RowVector
subclass that keeps its columns in a cudf::table on the device and is constructed with
no CPU children (childrenSize_ == 0). VeloxColumnarBatch::from hands these batches to
host-side code as-is, without ever copying the columns down from the GPU. Any CPU code that
then reads a child column crashes:

RowVector::childAt: index < childrenSize_ (N vs 0)
"Trying to access non-existing child in RowVector"

This was observed in two host side paths:

  • the value stream (RowVectorStream::next / CudfVectorStreamBase::next), e.g. when Spark
    samples a global ORDER BY via RangePartitioner and evaluates a projection over the
    sampled batch;
  • the broadcast hash-join build (JniHashTableHashTableBuilder::addInput), which reads
    join-key children directly.

This PR adds gluten::materializeVeloxRowVector() (cpp/velox/utils/CudfVectorUtils.h): for a
CudfVector it copies the real GPU columns into CPU-resident Velox children via
cudf_velox::with_arrow::toVeloxColumn() (the exact inverse of how the CudfVector was built —
nothing dropped or fabricated); for any other vector, and for non-GPU builds
(#ifndef GLUTEN_ENABLE_GPU), it returns the input unchanged. It is called immediately before
every host-side child access: RowVectorStream, CudfVectorStream, the JniHashTable build
loop, VeloxColumnarBatch::{ensureFlattened,compose,select,toUnsafeRow}, the columnar-to-row
prune in VeloxJniWrapper, and VeloxBatchResizer.

The change is inert for the CPU backend (compile-guarded + a runtime CudfVector type check →
no-op passthrough), so it does not affect non-GPU execution.

This is a correctness fix at the GPU→CPU boundary. Keeping these operators fully on the GPU (so
no materialization is needed) is separate follow-up work; likewise the broadcast-join CUDA
build/probe ordering race (Velox #17758), the cuDF expression gaps, and the CudfTopN
null-input crash are out of scope here.

How was this patch tested?

  • Added a unit test in cpp/velox/tests/VeloxGpuShuffleWriterTest.cc covering the CudfVector
    → host RowVector materialization path.
  • TPC-H SF10 with --decimal-as-double via gluten-it queries-compare (GPU output diffed
    against vanilla Spark, row and value), spark.gluten.sql.columnar.cudf=true, on
    2× NVIDIA L40S / CUDA 12.9.

Before this patch, the GPU path crashed with childAt (N vs 0) on every query whose plan
carried a CudfVector into a host-side operator (all global-ORDER BY and broadcast-join
queries). After this patch the crashes are gone and 12 of 22 queries are value-correct on the
GPU path
(output identical to vanilla Spark), with 2–4× speedups:

Fixed / value-correct (12): q1, q4, q5, q6, q7, q9, q11, q13, q14, q16, q21, q22
(e.g. q1 = 4 rows, q9 = 175, q16 = 27840, q21 = 100 — all matching vanilla).

Still failing — separate, out-of-scope follow-ups (10):

queries symptom cause (not addressed here)
q2, q18, q20 empty joins (0 rows) build→probe CUDA stream-ordering race (Velox #17758)
q8, q10, q12 wrong row counts same race (nondeterministic match cardinality)
q17, q19 wrong values cuDF expression-evaluation gaps → CPU fallback
q3, q15 crash CudfTopN::doAddInput null input

These are independent of the CudfVector materialization boundary and are tracked separately.
CPU Gluten (cudf=false) remains correct for all 22 and is unaffected by this change.

@github-actions github-actions Bot added the VELOX label Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant