Add GPU support for try_variant_get [databricks] - #15370
Conversation
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
5b078ef to
a1c47cb
Compare
|
build |
Greptile SummaryThis PR adds GPU acceleration for Spark 4.x
Confidence Score: 5/5Safe to merge; GPU/CPU results are validated by 31 integration tests and the incompat flag gates rollout. The core extraction logic is well-tested across boundary values, null rows, missing fields, heterogeneous batches, and every declared fallback path. Resource management follows withResource/safeMap throughout. The two findings are minor defensive gaps that do not affect correctness or runtime behaviour of supported paths. Files Needing Attention: The mixed-type child routing in GpuVariantGet.withCudfVariantView and the shim annotation in GpuColumnVectorVariantSuite are the only spots worth a second look before merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant Exec as ProjectExec (GPU)
participant Meta as GpuVariantGetMeta
participant Expr as GpuVariantGet.doColumnar
participant cuDF as VariantUtils (cuDF)
participant Bridge as GpuCpuBridgeExpression
Meta->>Meta: tagExprForGpu()
Meta->>Expr: convertToGpu(lhs, rhs)
Exec->>Expr: doColumnar(input: GpuColumnVector)
Expr->>Expr: "getChildColumnView(0=value, 1=metadata)"
Expr->>Expr: withCudfVariantView
Expr->>cuDF: getVariantFieldValue(cudfVariant, path)
cuDF-->>Expr: rawValue ColumnView
alt StringType target
Expr->>cuDF: castVariantValue(rawValue, STRING)
Expr->>Expr: allRowsCovered?
alt covered
Expr-->>Exec: GPU column
else needs coercion
Expr->>Bridge: columnarEval
Bridge-->>Exec: CPU column
end
else Integer targets
Expr->>cuDF: castVariantValue x4 INT8/16/32/64
Expr->>Expr: allRowsCovered?
alt covered
Expr->>Expr: normalizeIntegers + narrowInteger
Expr-->>Exec: GPU column
else needs coercion
Expr->>Bridge: columnarEval
Bridge-->>Exec: CPU column
end
end
Reviews (8): Last reviewed commit: "Merge branch 'main' into variant-extract..." | Re-trigger Greptile |
| def isVariantCudfAvailable: Boolean = { | ||
| try { | ||
| val variantUtils = Class.forName("ai.rapids.cudf.VariantUtils", true, | ||
| Thread.currentThread().getContextClassLoader) | ||
| variantUtils.getMethod("getVariantFieldValue", classOf[ColumnView], classOf[String]) | ||
| variantUtils.getMethod("castVariantValue", classOf[ColumnView], classOf[DType]) | ||
| variantUtils.getMethod("extractVariantField", classOf[ColumnView], classOf[String], | ||
| classOf[DType]) | ||
| true | ||
| } catch { | ||
| case _: ClassNotFoundException | _: NoSuchMethodException | _: LinkageError => false | ||
| } | ||
| } |
There was a problem hiding this comment.
Availability check validates unused methods
isVariantCudfAvailable verifies the existence of getVariantFieldValue and castVariantValue, but neither method is called anywhere in this file — only extractVariantField is actually invoked at runtime. If a future cuDF release renames or removes those two helper methods while keeping extractVariantField, the entire feature silently falls back to CPU even though the required API is present. The guard should only check the method the code actually calls.
| private def isVariantPhysicalType(fileType: Type): Boolean = { | ||
| if (fileType.isPrimitive || fileType.asGroupType().getFieldCount != 2) { | ||
| false | ||
| } else { | ||
| val groupType = fileType.asGroupType() | ||
| Seq("value", "metadata").zipWithIndex.forall { case (name, index) => | ||
| val field = groupType.getType(index) | ||
| field.getName == name && | ||
| field.isRepetition(Type.Repetition.REQUIRED) && | ||
| field.isPrimitive && | ||
| field.asPrimitiveType().getPrimitiveTypeName == PrimitiveTypeName.BINARY | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Strict
REQUIRED check may reject valid Variant files
isVariantPhysicalType rejects any Parquet file where value or metadata is encoded with OPTIONAL repetition. Non-Spark Variant writers (e.g., Delta Lake connectors, Arrow-based tools) may write these fields as OPTIONAL BINARY — still spec-compliant Variant — which would fail this check and cause an incompatibility error or unexpected CPU fallback. Consider accepting OPTIONAL repetition as well, or at minimum document why REQUIRED is the only valid encoding here.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
f172901 to
a1c47cb
Compare
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
…pport Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
|
NOTE: release/26.08 has been created from main. Please retarget your PR to release/26.08 if it should be included in the release. |
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
…pport Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
|
build |
|
@greptile-apps review the PR again. |
Contributes to #15220 and #15221.
Description
This PR adds GPU support for Spark 4.x Variant extraction from Parquet reads.
It accelerates non-strict
try_variant_getwhen:$.fieldor$.nested.field.tinyint,smallint,int,bigint, orstring.The implementation uses the cuDF Java
VariantUtils.getVariantFieldValueandVariantUtils.castVariantValueAPIs added in rapidsai/cudf#23069. Exact string and signed-integer Variant values are decoded on GPU. Batches containing values that require Spark coercion semantics, such as string-to-integer conversion, are evaluated through the CPU bridge to preserve Spark-compatible results.Strict
variant_get, non-literal paths, array paths, quoted-key paths, and unsupported target types continue to fall back to CPU. Variant writes remain unsupported on GPU.Changes
try_variant_getacross Spark 4.0+ shims, including Databricks 17.3.tinyint,smallint,int,bigint, andstringtargets.LIST<UINT8>andSTRINGVariant children during host conversion.Configuration
Variant extraction is currently registered as an incompatible operation and requires:
The CPU bridge used for Spark-compatible runtime coercions is enabled by default.
Testing
variant_test.py: 31 passed.400db173) build and focused tests: passed.Performance
Tested on a Tesla T4 with OSS Spark 4.0.0 using 10 million rows, 128 Parquet partitions,
local[4], and 6 GB driver memory. Each result is the average of three measured runs after one warm-up run.$.idasBIGINT$.nameasSTRINGProjection queries used Spark's
noopsink.Benchmark queries
A nested integer filter followed by
COUNT(*)achieved a 1.51x speedup. DirectSUM(try_variant_get(...))did not improve performance because its partial aggregate remains on CPU when the input schema contains the rawVariantType; the Variant extraction and final aggregate still run on GPU.Checklists
Documentation
Testing
Performance