feat: accelerate scalar Python UDFs with CometArrowEvalPythonExec - #5744
Open
andygrove wants to merge 4 commits into
Open
feat: accelerate scalar Python UDFs with CometArrowEvalPythonExec#5744andygrove wants to merge 4 commits into
andygrove wants to merge 4 commits into
Conversation
Extend Comet's opt-in Arrow UDF path (spark.comet.exec.pyarrowUDF.enabled) from mapInArrow/mapInPandas to ArrowEvalPythonExec, the operator Spark uses for scalar Python UDFs: an Arrow-optimized udf(), a pandas_udf, and Spark 4.1's arrow_udf. CometArrowEvalPythonExec sends the UDF argument columns straight from the input batch to the Python worker and appends the returned columns to the input batch's own columns, removing Spark's per-row HybridRowQueue and its spill path, the MutableProjection over the arguments, and the JoinedRow plus UnsafeProjection on the way back, along with the ColumnarToRow a Comet child would otherwise force. The input batch is deep-copied on the way in. It has to outlive the child iterator's advance because the Python runner writes from its own thread, and Comet's native operators recycle the buffers behind consecutive batches. The copy is a bulk memcpy per Arrow buffer and replaces Spark's per-row UnsafeRow copy. Scope of this first version, enforced in the shim so unsupported shapes fall back to vanilla Spark rather than failing: - Non-iterator eval types only. The iterator variants guarantee only equal total row counts, not equal batching, which the batch pairing relies on. - Every UDF argument must be a plain attribute of the child. This also excludes chained UDFs and keyword arguments, neither of which is an Attribute. - Pickled BatchEvalPythonExec stays out of scope: there is no columnar boundary to preserve. Also extends CometArrowPythonRunnerBase to write a flat input stream, since scalar eval types exchange one top-level column per argument rather than a single struct, and lifts the shared runner-input resolution into ShimPythonRunnerInputs so a class can mix in both Python operator shims. Closes apache#5386
`ensure-jars-have-correct-contents.sh` allowlists `org/apache/spark/sql/execution/python/CometArrowPythonRunner.*`, which does not match the new `CometArrowEvalPythonRunner`, so the `check-jar-contents` integration-test step rejected the jar.
- Share the dictionary-decode step between `CometBatchDeepCopy` and `ColumnarBatchArrowReader`, which had duplicated ~12 lines of it, as `CometArrowVectors.materialize`. - Build the copied record batch with `retainBuffers = false` so it adopts the references `copyRecordBatch` creates instead of adding its own. That drops a retain/release cycle per buffer and the swallow-all cleanup loop that existed only to undo the extra retain, and lets the method return the batch rather than take a continuation. Failure cleanup now uses `AutoCloseables`, as `NativeUtil` does. - Wrap only the UDF argument columns when building the batch sent to the Python worker, rather than wrapping every column of the retained root and indexing into the result. Saves N-M `CometVector` allocations per batch, and drops an array-as-function idiom that read like a typo. - Compute the per-UDF argument offsets once in `resolveArrowEvalPythonArgs` instead of appending them inside a `forall` predicate whose result is discarded when resolution fails. - Move `ShimCometArrowEvalPython` to the shared `spark-4.1+` source root; the 4.1 and 4.2 copies were byte-identical. - Share the stub `PythonFunction` between the two plan-rule suites, and replace three copies of save/set/restore conf scaffolding in the pytest module with a `temp_conf` context manager in `conftest.py`.
`SQL_ARROW_BATCHED_UDF` is the one eval type whose worker may read an input schema before the UDF payload, but only Spark 4.1 actually does. Spark 4.0's worker goes straight to the profiler flag, and Spark 4.2 derives the types from the UDFs themselves; both of their own `ArrowPythonWithNamedArgumentRunner` implementations write nothing there. Comet wrote it on all three, so on 4.0 and 4.2 the worker read the schema string's bytes as the profiler flag and the UDF count, desynchronized, and then blocked forever waiting for input that never came. The CI jobs for those two versions hung until cancelled while 4.1 passed. Verified against a real Python worker: the scalar UDF suite now passes on Spark 4.0, and 4.1 is unaffected. Spark 4.2 could not be checked locally -- pyspark 4.2.0 hangs on this machine even with Comet entirely absent, on both Python 3.13 and 3.14 -- so CI is the check for it. Also cap the job at 45 minutes. A worker protocol mismatch blocks rather than failing, so the hung jobs would otherwise have run to GitHub's 6-hour default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #5386.
Rationale for this change
Comet's opt-in Arrow UDF path (
spark.comet.exec.pyarrowUDF.enabled) covered onlymapInArrowandmapInPandas. Scalar Python UDFs fell back entirely, so a Comet scan feeding one paid aColumnarToRowtransition and Spark then re-encoded those rows back into Arrow to reach the Python worker.One operator,
ArrowEvalPythonExec, covers three user-facing UDF families, becauseExtractPythonUDFsroutes all of them to it:udf()whenspark.sql.execution.pythonUDF.arrow.enabled=true@pandas_udf(scalar)@arrow_udfThe first is what makes ordinary PySpark UDF code benefit from Comet, so this is the operator that turns "Comet accelerates PyArrow UDFs" into "Comet accelerates Python UDFs".
The win is larger than it was for
mapInArrow, becauseEvalPythonEvaluatorFactoryis considerably more row-bound thanMapInBatchExecwas. Per row the vanilla path does aHybridRowQueue.add(UnsafeRow)(a full row copy that can spill to disk), aMutableProjectionto materialize the UDF arguments, then on the way back aqueue.remove(), aJoinedRow, and anUnsafeProjectionover the full output schema.What changes are included in this PR?
CometArrowEvalPythonExecsends the UDF argument columns straight from the input batch to aCometArrowEvalPythonRunner, and appends the columns the worker returns to the input batch's own columns to form the output batch. No row is materialized, and the row queue and its spill path are gone.One design point worth flagging, since it differs from what the issue proposed. The issue suggested retaining the input
ColumnarBatchand appending to it. That is not safe:CometExecIterator.hasNextcloses the previous batch before fetching the next one, precisely because "the buffer memory [is] shared across batches in the native side", and the Python runner writes from its own thread and pipelines ahead. So the input batch is deep-copied on the way in (CometBatchDeepCopy) — a bulkmemcpyper Arrow buffer, with dictionary columns decoded first. That copy replaces Spark's per-rowUnsafeRowcopy into a spillable queue, so it stays well ahead of the unoptimized path, but it is a copy rather than the zero-copy the issue envisaged.Scope of this first version. Each restriction is enforced in the shim's matcher, so an unsupported shape falls back to vanilla Spark rather than failing:
SQL_ARROW_BATCHED_UDF,SQL_SCALAR_PANDAS_UDF, andSQL_SCALAR_ARROW_UDFon 4.1+). The iterator variants guarantee only equal total row counts, not equal batching, which the batch pairing relies on.udf(col("a"))qualifies butudf(col("a") + 1)does not. This restriction also excludes chained UDFs (f(g(x)), folded into one operator whose argument is aPythonUDF) and keyword arguments (NamedArgumentExpression), neither of which is anAttribute.BatchEvalPythonExecstays out of scope, as the issue specified: per-row boxing, pickle, and interpreter cost dominate, and the unpickled Java-object results would have to be converted into Arrow vectors to keep the batch all-Comet.Comet does not flip
spark.sql.execution.pythonUDF.arrow.enabledon the user's behalf, since that conf changes Spark's own type coercion and error semantics rather than just the transport. The user guide now explains what it does and why enabling it is a prerequisite for accelerating a plainudf().Supporting changes:
CometArrowPythonRunnerBasegains awrapInputInStructhook. Scalar eval types exchange one top-level column per argument, matching the flat_0,_1, ... schema Spark builds, rather than the single structmapInArrowuses. The runner writes the input schema JSON forSQL_ARROW_BATCHED_UDFand uses theArgumentMetadataform ofwriteUDFs, both of which the worker's protocol requires for these eval types.Spark4xMapInBatchSupportintoShimPythonRunnerInputs, so a class can mix in both Python operator shims without a conflictingRunnerInputs.EliminateRedundantTransitionsrewrites the operator behind the existing feature flag, annotates it with the same[COMET-INFO]opt-in hint when the flag is off, and honours the sameuseLargeVarTypesfallback.extractColumnarChildalso accepts aCometArrowEvalPythonExecchild, so stacked operators stay columnar end to end.spark/acceleratedpytest fixtures move intoconftest.pyso both UDF test modules run against one Spark session.How are these changes tested?
CometArrowEvalPythonSuite(11 tests, new) covers the plan rule without spinning up Python: the rewrite and its output attributes, transition stripping, each supported eval type, and a negative case for every fallback condition (iterator eval type, non-attribute argument, chained UDF,useLargeVarTypes, feature flag off, and the opt-in hint), plus stacked operators.test_scalar_python_udf.py(29 tests, new) runs a real Python worker end to end, each test in both accelerated and fallback modes: all three UDF families, pass-through of a wide mixed-type child, nulls, multiple arguments, a repeated argument that must deduplicate to one exchanged column, several UDFs in one operator, stacked operators of different eval types (where the outer UDF's argument is the column the inner worker produced), empty input, and a multi-batch run withmaxRecordsPerBatch=100. The fallback cases assert the operator is left to Spark and still produces correct results.CometArrowPythonRunnerSuitegains a test that the flat path emits the columns as top-level fields with no struct field node or validity buffer prepended, and allocates nothing of its own.Locally: all 162 pytest cases across both modules pass against Spark 4.1.3; the 28 JVM tests in the three suites pass;
CometExecSuite+CometSparkSessionExtensionsSuite(155 tests) pass as a regression check on the modified plan rule; andtest-compileis clean on all five Spark profiles (3.4, 3.5, 4.0, 4.1, 4.2). CI runs the pytest modules against a real worker on 4.0, 4.1, and 4.2, whose runner constructor and command framing differ.