kazantsev-maksim commented on PR #4744:
URL:
https://github.com/apache/datafusion-comet/pull/4744#issuecomment-5574956971
## Removed LambdaParamsCapture
The `LambdaParamsCapture` / `pin_unused_params` wrapper in the native code
is gone
entirely. It existed to anchor unused lambda parameters in the expression
tree so
DataFusion's optimizer could not prune them and break the runtime batch
layout.
This workaround became unnecessary after apache/datafusion#24162 landed in
DataFusion 55: `LambdaExpr` now computes `used_param_indices()` itself, and
`LambdaArgument::new` pushes only the parameters actually referenced by the
body
into the evaluation batch (`captures ++ used_params`, in declaration order).
The
runtime layout contract is now enforced on the DataFusion side, so the
planner
assigns lambda variables their declared positions and DataFusion compacts
them -
no anchoring wrapper needed. Nested-lambda scoping (the `exprId`-keyed scope
stack in `lambda.rs`) is unaffected and still handles shadowing.
Verified: three levels of nesting, an inner HOF whose value argument is an
outer
lambda variable, sibling nested HOFs, an inner lambda referencing an outer
variable, and multi-param lambdas with unused parameters all match Spark.
## Benchmark results
Simple benchmark result (Apple M1 Pro, OpenJDK 17.0.19, 2 iterations, single
run
on final code; dispatch-path selection verified via serde logging):
| Benchmark | Spark (ms) | Comet Native (ms) |
Comet Codegen (ms) | Native vs Spark | Native vs Codegen |
|--------------------------------------|-----------:|------------------:|-------------------:|------------------:|
| int literal | 5156 | 1408 |
5234 | 3.7x | 3.7x |
| capture outer column | 6582 | 1449 |
5147 | 4.5x | 3.6x |
| compound predicate (AND / range) | 8659 | 1498 |
7413 | 5.8x | 4.9x |
| arithmetic expression in lambda | 8826 | 1545 |
7494 | 5.7x | 4.8x |
| string length predicate | 20524 | 2579 |
17323 | 8.0x | 6.7x |
| string equality comparison | 12657 | 3051 |
8896 | 4.1x | 2.9x |
| array with nulls (IS NOT NULL check) | 8703 | 1798 |
6855 | 4.8x | 3.8x |
| nested array (size check) | 3752 | 1028 |
2121 | 3.6x | 2.1x |
| chained filters (pipeline) | 10115 | 1898 |
15495 | 5.3x | 8.2x |
| short arrays (JNI overhead) | 933 | 203 |
627 | 4.6x | 3.1x |
| large arrays (SIMD) | 63915 | 14675 |
54114 | 4.4x | 3.7x |
The native path is 3.6-8.0x faster than vanilla Spark and 2.1-8.2x faster
than
the JVM codegen dispatch path across all scenarios. The codegen dispatch path
(running Spark's own lambda evaluation inside the Comet kernel) is on par
with
or slower than vanilla Spark on lambda-heavy queries (chained filters: 0.7x),
confirming the per-batch JNI and row-wise evaluation overhead this PR
removes.
@comphead @andygrove Could you please take another look?
--
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]
