viirya commented on PR #6071: URL: https://github.com/apache/datafusion-comet/pull/6071#issuecomment-5784102784
@andygrove I completed the default-settings TPC applicability measurements and a follow-up to explain the zero registry hits. This used Spark 4.1.3, `local[4]`, and SF=1 Parquet data. Apart from enabling sharing and configuring Comet/local resources, AQE, native scans, shuffle mode (`auto`), direct shuffle reads, and the initial 200 shuffle partitions retained their defaults. All 22 TPC-H queries and 103 TPC-DS query variants completed. | Suite | Queries with an eligible block | Eligible native plan invocations / all invocations | Distinct eligible scopes | Registry hits | |---|---:|---:|---:|---:| | TPC-H | 11/22 | 13/436 (3.0%) | 13 | 0 | | TPC-DS | 17/103 | 32/3,415 (0.94%) | 32 | 0 | The invocation denominator counts task-level JNI `createPlan` calls, including private-only native shuffle writers; it is not a count of distinct Spark operators or Spark tasks. There are two related limitations: - Most native blocks fail sharing admission. Among rejected invocations, 384/423 for TPC-H and 2,700/3,383 for TPC-DS contain at least one of `NativeScan`, `ShuffleScan`, or `ShuffleWriter` (union counts, without double-counting overlaps). Admission applies recursively to the entire native block, so supported operators above an excluded input still use a private native plan. Other restrictions include expression forms and operator features such as Top-K. - Every eligible scope had only one invocation. I reran the 28 queries with eligible blocks and recorded the native `partition_count`: **all 45 eligible blocks had exactly one partition**, again with zero hits. For example, TPC-H Q1 admits its final sort above a coalesced AQE shuffle read, while the preceding scan/aggregate/shuffle work remains private. Thus the query-level eligibility percentages do not translate into useful sharing in this run. An idle gap in the weak registry is not needed to explain these misses. The 28 queries with eligible blocks also passed result comparisons with sharing disabled. SF=1 is a limitation: larger data can change AQE partitioning and join choices, so these results do not establish zero reuse at every scale. They do establish that this default-settings run provides no evidence of a TPC benefit. The sharing-only microbenchmarks after decoded-cache removal still show a benefit for the controlled 64-column projection/filter workload (about 54% lower task CPU and 12% lower elapsed time), but the 1/16/64-expression SUM workloads show no clear speedup. The upstream DataFusion metrics API PR has now been approved; integrating it and rerunning the large-stage test remains separate outstanding work. It will not change these admission restrictions or single-partition scopes. **Would improving useful TPC coverage under default settings be a merge requirement for this PR?** Assuming the metrics issue and other outstanding review items are addressed, would the current limited, default-off scope be acceptable with coverage expansion in follow-up work, or should we expand it here first? If wider TPC coverage is required before merging, it would help to agree on the target workloads/scale and expected multi-partition reuse. We would need additional time to implement and validate the excluded scan/shuffle paths and relevant expressions, including task-specific input binding and execution-state correctness. I would like to align on that scope before extending the PR further. -- 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]
