sam-1112 commented on PR #7443: URL: https://github.com/apache/datafusion-comet/pull/7443#issuecomment-6607060553
Follow-up diagnostics point to **query RowFilter/cache overhead in Candidate** as the main suspect for the random decimal-only regression. Profiles show additional cache data movement and type-conversion work; decimal comparison itself did not show a corresponding increase. I use the following three build names consistently below: | Build | Definition | | --- | --- | | **Main** | Official Main at `383ec5627c61577e1557e63cb32b025849c6c2c6`, without this PR's decimal predicate pushdown. | | **Candidate** | The same Main commit plus this PR's runtime changes, with decimal metadata pruning and query RowFilter enabled. | | **Query RowFilter OFF** | Candidate plus a diagnostic change that disables only query RowFilter. Metadata pruning, the complete post-scan filter and Iceberg delete processing remain enabled. This build is outside the PR. | Here, **query RowFilter** means reader-level evaluation of the query predicate; it does not include filtering required for Iceberg deletes. **Metadata pruning** means row-group and page pruning. The key results are median query latencies: | Workload | Main | Candidate | Query RowFilter OFF | | --- | ---: | ---: | ---: | | Random 10%, decimal-only | 38.10 ms | 50.34 ms | 38.68 ms | | Sorted 0.1%, decimal + payload | 69.52 ms | 22.46 ms | 20.92 ms | | Random 0.1%, decimal + 1 KiB payload | — | 94.22 ms | 295.36 ms | All three summary workloads use DECIMAL(18,2). The first two rows come from the same three-build experiment; the third comes from a separate workload experiment without a Main measurement. Query RowFilter OFF retained approximately **98% fewer reader bytes than Main** for the sorted payload query. However, Candidate was approximately **3.1× faster than Query RowFilter OFF** for Random 0.1% with a 1 KiB payload. These local, warm-cache, uncompressed measurements suggest that query RowFilter can add overhead to narrow projections but avoid substantial payload work in selective, wide queries. They do not establish a general switching threshold. Query RowFilter OFF remains a diagnostic build outside this PR. Would it make sense to first open an iceberg-rust issue to discuss independent control of metadata pruning and query RowFilter, then create a linked Comet issue to track the end-to-end performance tradeoffs and follow-up integration? The Comet tracker could be opened while the upstream discussion is ongoing, keeping this PR focused on decimal predicate pushdown. <details> <summary>Full profiling and RowFilter diagnostic results, methodology and limitations</summary> I investigated the random decimal-only regression with CPU profiling and a separate query RowFilter experiment. These results are separate from the 48-case Main vs Candidate benchmark matrix in the PR description. Query RowFilter OFF is an experimental build outside this PR. ### CPU profiling: random DECIMAL(18,2), 10% matching Main was pinned to `383ec5627c61577e1557e63cb32b025849c6c2c6`; Candidate applied this PR's runtime changes to that same commit. The Main and Candidate release libraries and data were unchanged from the original benchmark. The query projected only the decimal column through `count`, `min` and `max`, over four million randomly ordered rows. Four independent JVMs ran Main → Candidate → Candidate → Main. Each JVM had 300 warmups and 800 measured queries, with macOS async-profiler `itimer` sampling at 1 ms and native frame-pointer stacks. | Metric | Main | Candidate | | --- | ---: | ---: | | Median latency | 34.03 ms | 43.82 ms | | Mean latency | 34.63 ms | 44.91 ms | | Process CPU time/query | 52.70 ms | 62.89 ms | | Reader bytes/query | 34,960,601 | 34,960,601 | | Scan output rows | 4,000,000 | 400,000 | | JVM | Measured queries | Median latency (ms) | Mean latency (ms) | Process CPU/query (ms) | | --- | --- | --- | --- | --- | | Main r1 | 800 | 33.62 | 34.34 | 51.57 | | Candidate r1 | 800 | 43.76 | 44.95 | 63.55 | | Candidate r2 | 800 | 43.88 | 44.86 | 62.22 | | Main r2 | 800 | 34.33 | 34.92 | 53.82 | The Candidate was 28.7% slower despite emitting fewer scan rows. Main and Candidate read the same number of bytes. Process CPU time, summed across threads, also increased. The largest additional sampled path in Candidate was `CachedArrayReader::consume_batch → memmove`, followed by `coerce_i64` in the predicate read path. | Sampled path | Main samples/100 queries | Candidate samples/100 queries | | --- | --- | --- | | All sampled stacks | 3898.62 | 4717.75 | | PredicateConverter callback | 0.00 | 21.19 | | Arrow decimal comparison kernel | 21.75 | 18.38 | | Page-index decode/evaluation | 6.25 | 29.12 | | Row-group metrics evaluation | 0.00 | 0.38 | | Predicate read-plan path | 0.00 | 240.75 | | RowSelection::from_filters | 0.00 | 43.12 | | CachedArrayReader, inclusive | 0.00 | 814.25 | | Cache → memmove, predicate + output phases | 0.00 | 635.75 | | Output-phase cache → memmove | 0.00 | 630.25 | | coerce_i64 | 12.50 | 140.69 | | JIT compiler | 768.06 | 715.75 | | G1 GC | 153.00 | 161.12 | | Stacks containing unknown frames | 92.31 | 105.94 | | Parquet reader, inclusive | 782.44 | 1443.12 | | Literal/Datum/fastnum path | 0.00 | 2.00 | These are inclusive sample counts, not function timings; categories overlap and cannot be added. The observations point to reader cache/data-movement and conversion work as likely contributors, rather than decimal comparison itself. They do not prove repeated decoding or assign an exact latency cost to individual functions. macOS ITIMER sampling also has signal-distribution limitations. ### Diagnostic experiment: retain metadata pruning, disable only query RowFilter - **Candidate:** the PR runtime changes, with decimal metadata pruning and query RowFilter enabled. - **Query RowFilter OFF:** Candidate plus a diagnostic iceberg-rust change with metadata and row-level predicates explicitly separated. Only query row-level filtering is disabled. Row-group/page pruning, equality-delete RowFilters, positional deletes and the complete post-scan filter remain enabled. This was not implemented by removing all `with_row_filter` calls. Six independent JVMs ran Main → Candidate → Query RowFilter OFF → Query RowFilter OFF → Candidate → Main, with 200 warmups and 400 measured iterations per case per JVM. The following DECIMAL(18,2) results report median (IQR), in milliseconds, and include profiling overhead: | Workload | Main | Candidate | Query RowFilter OFF | | --- | ---: | ---: | ---: | | Random, 10% matching, decimal-only | 38.10 (3.26) | 50.34 (3.73) | 38.68 (4.71) | | Sorted, 0.1% matching, decimal-only | 33.40 (3.08) | 22.96 (3.43) | 21.94 (2.46) | | Sorted, 0.1% matching, decimal + payload | 69.52 (4.77) | 22.46 (3.36) | 20.92 (2.97) | | Workload | Build | Mean latency (ms) | Process CPU/query (ms) | Reader bytes | Scan output rows | Matching rows | | --- | --- | --- | --- | --- | --- | --- | | Random 10%, decimal-only | Main | 38.82 | 63.35 | 34,960,601 | 4,000,000 | 400,000 | | Random 10%, decimal-only | Candidate | 50.96 | 75.21 | 34,960,601 | 400,000 | 400,000 | | Random 10%, decimal-only | Query RowFilter OFF | 39.80 | 64.25 | 34,960,601 | 4,000,000 | 400,000 | | Sorted 0.1%, decimal-only | Main | 33.74 | 48.96 | 34,960,637 | 4,000,000 | 4,000 | | Sorted 0.1%, decimal-only | Candidate | 23.36 | 37.19 | 2,976,598 | 4,000 | 4,000 | | Sorted 0.1%, decimal-only | Query RowFilter OFF | 23.06 | 37.68 | 2,931,197 | 5,633 | 4,000 | | Sorted 0.1%, decimal + payload | Main | 70.49 | 83.56 | 179,213,079 | 4,000,000 | 4,000 | | Sorted 0.1%, decimal + payload | Candidate | 22.65 | 34.99 | 3,137,690 | 4,000 | 4,000 | | Sorted 0.1%, decimal + payload | Query RowFilter OFF | 21.90 | 34.06 | 3,155,112 | 5,633 | 4,000 | Query RowFilter OFF returned the random decimal-only query close to the concurrently measured Main baseline. Process CPU time/query was Main 63.35 ms, Candidate 75.21 ms and Query RowFilter OFF 64.25 ms. The cache/memmove sampled paths present in Candidate were not observed in Query RowFilter OFF. The absolute timings differ from the earlier profile run; the comparison should be made within each run. Query RowFilter OFF also retained 91.6% and 98.2% reductions in reader bytes for the two sorted cases. A separate single-row-group fixture confirmed that page pruning remained effective: Query RowFilter OFF's full/narrow reads were 9,343,497/582,049 bytes, while Main read 9,343,497 bytes for both. The sorted-query speedups above include all metadata pruning, rather than isolating page pruning's latency contribution. ### Cross-workload coverage: all 18 Candidate vs Query RowFilter OFF cases, including the full-scan control I then tested one million rows with distinct 1 KiB ASCII payloads, using `count(amount)` and `sum(length(payload))` so payload must be projected while the result remains small. Files were uncompressed with dictionary encoding disabled. These measurements did not attach a CPU profiler. The tables combine the complete aggregated results from multiple Candidate → Query RowFilter OFF → Query RowFilter OFF → Candidate measurement batches, including the decimal-only coverage and the unfiltered control. Decimal-only coverage used four million rows; the wide projections used one million rows. Compare Candidate with Query RowFilter OFF within each row; differences between rows do not isolate a single variable. IQR is Q3 − Q1. Process CPU time sums all process threads; reader bytes are the cumulative `bytes_scanned` metric. Earlier priority/coverage JVMs ran multiple workloads; later payload-priority/payload-paths batches used independent JVMs per query. The earlier full-scan control had 20 warmups and 40 measurements per JVM; other earlier cases and payload-priority cases had 100/200, and payload-paths cases had 50/100. Per-build measured totals are shown below. No-page-index fixtures removed column/offset index references while retaining data pages and row-group statistics, and were registered without manifest column bounds. | Workload | Samples/build | Candidate median (IQR), ms | Query RowFilter OFF median (IQR), ms | Candidate mean, ms | Query RowFilter OFF mean, ms | | --- | --- | --- | --- | --- | --- | | Random 0.1%, DECIMAL(18,2) + 1 KiB payload | 400 | 94.22 (10.57) | 295.36 (15.09) | 97.25 | 300.62 | | Random 1%, DECIMAL(18,2) + 1 KiB payload | 400 | 306.33 (62.59) | 301.48 (12.40) | 319.64 | 306.40 | | Random 10%, DECIMAL(18,2) + 1 KiB payload | 400 | 480.15 (28.07) | 360.80 (60.39) | 481.62 | 376.33 | | Random full scan, DECIMAL(18,2) + 1 KiB payload (control) | 80 | 393.98 (55.48) | 455.91 (42.88) | 404.30 | 458.93 | | Sorted 10%, DECIMAL(18,2) + 1 KiB payload | 400 | 95.43 (6.30) | 79.88 (8.51) | 97.91 | 82.13 | | Random 0.1%, DECIMAL(38,2) + 1 KiB payload | 200 | 110.28 (6.44) | 303.76 (12.52) | 112.33 | 309.14 | | Sorted 10%, DECIMAL(38,2) + 1 KiB payload | 200 | 101.41 (7.71) | 87.19 (7.61) | 103.93 | 89.41 | | No page index, Random 0.1%, DECIMAL(18,2) + 1 KiB payload | 200 | 262.82 (10.41) | 293.11 (15.71) | 268.01 | 296.24 | | No page index, Sorted 10%, DECIMAL(18,2) + 1 KiB payload | 200 | 94.44 (6.48) | 82.22 (8.39) | 96.71 | 83.99 | | Sorted 10%, DECIMAL(18,2)-only | 400 | 31.69 (4.31) | 31.02 (4.02) | 32.75 | 32.04 | | Random 0.1%, DECIMAL(38,2)-only | 400 | 70.95 (4.43) | 66.45 (4.06) | 73.02 | 67.07 | | Random 10%, DECIMAL(38,2)-only | 400 | 81.95 (4.79) | 64.75 (3.05) | 85.25 | 65.52 | | Sorted 0.1%, DECIMAL(38,2)-only | 400 | 24.01 (3.77) | 22.74 (3.38) | 24.65 | 23.40 | | Sorted 10%, DECIMAL(38,2)-only | 400 | 27.96 (3.85) | 26.11 (3.03) | 28.70 | 26.46 | | No page index, Random 0.1%, DECIMAL(18,2)-only | 400 | 32.41 (3.10) | 31.59 (2.79) | 33.01 | 32.28 | | No page index, Random 10%, DECIMAL(18,2)-only | 400 | 41.85 (3.16) | 32.03 (2.63) | 42.32 | 32.14 | | No page index, Sorted 0.1%, DECIMAL(18,2)-only | 400 | 20.41 (2.97) | 19.51 (3.12) | 20.55 | 20.15 | | No page index, Sorted 10%, DECIMAL(18,2)-only | 400 | 23.04 (3.41) | 20.80 (3.04) | 23.41 | 21.35 | | Workload | Candidate CPU/query, ms | Query RowFilter OFF CPU/query, ms | Candidate reader bytes | Query RowFilter OFF reader bytes | Candidate scan rows | Query RowFilter OFF scan rows | Matching rows | | --- | --- | --- | --- | --- | --- | --- | --- | | Random 0.1%, DECIMAL(18,2) + 1 KiB payload | 152.34 | 346.30 | 364,465,603 | 1,052,962,455 | 1,000 | 1,000,000 | 1,000 | | Random 1%, DECIMAL(18,2) + 1 KiB payload | 364.03 | 352.11 | 1,037,868,282 | 1,052,962,455 | 10,000 | 1,000,000 | 10,000 | | Random 10%, DECIMAL(18,2) + 1 KiB payload | 504.68 | 401.14 | 1,052,898,653 | 1,052,962,455 | 100,000 | 1,000,000 | 100,000 | | Random full scan, DECIMAL(18,2) + 1 KiB payload (control) | 430.31 | 490.78 | 1,052,962,455 | 1,052,962,455 | 1,000,000 | 1,000,000 | 1,000,000 | | Sorted 10%, DECIMAL(18,2) + 1 KiB payload | 144.47 | 128.96 | 120,438,447 | 122,377,077 | 100,000 | 102,072 | 100,000 | | Random 0.1%, DECIMAL(38,2) + 1 KiB payload | 177.43 | 367.05 | 367,835,372 | 1,061,151,760 | 1,000 | 1,000,000 | 1,000 | | Sorted 10%, DECIMAL(38,2) + 1 KiB payload | 159.80 | 149.71 | 121,504,408 | 122,966,755 | 100,000 | 101,611 | 100,000 | | No page index, Random 0.1%, DECIMAL(18,2) + 1 KiB payload | 326.93 | 354.06 | 1,040,567,863 | 1,040,567,863 | 1,000 | 1,000,000 | 1,000 | | No page index, Sorted 10%, DECIMAL(18,2) + 1 KiB payload | 155.58 | 141.36 | 112,394,823 | 112,394,823 | 100,000 | 104,403 | 100,000 | | Sorted 10%, DECIMAL(18,2)-only | 66.45 | 66.11 | 6,149,908 | 6,104,516 | 400,000 | 401,376 | 400,000 | | Random 0.1%, DECIMAL(38,2)-only | 93.06 | 86.49 | 67,257,224 | 67,257,224 | 4,000 | 4,000,000 | 4,000 | | Random 10%, DECIMAL(38,2)-only | 101.37 | 82.10 | 67,257,224 | 67,257,224 | 400,000 | 4,000,000 | 400,000 | | Sorted 0.1%, DECIMAL(38,2)-only | 38.77 | 36.02 | 3,390,444 | 3,196,582 | 4,000 | 5,580 | 4,000 | | Sorted 10%, DECIMAL(38,2)-only | 40.42 | 39.25 | 9,721,859 | 9,542,881 | 400,000 | 401,296 | 400,000 | | No page index, Random 0.1%, DECIMAL(18,2)-only | 45.63 | 46.44 | 33,123,149 | 33,123,149 | 4,000 | 4,000,000 | 4,000 | | No page index, Random 10%, DECIMAL(18,2)-only | 51.51 | 40.83 | 33,123,149 | 33,123,149 | 400,000 | 4,000,000 | 400,000 | | No page index, Sorted 0.1%, DECIMAL(18,2)-only | 28.01 | 27.14 | 2,340,727 | 2,340,727 | 4,000 | 161,143 | 4,000 | | No page index, Sorted 10%, DECIMAL(18,2)-only | 33.75 | 31.39 | 4,925,026 | 4,925,026 | 400,000 | 483,429 | 400,000 | For Random 0.1% with DECIMAL(18,2), Candidate was approximately 3.13× faster, read 364,465,603 versus 1,052,962,455 bytes, and emitted 1,000 versus 1,000,000 scan rows. DECIMAL(38,2), using actual 38-digit coefficients, showed the same direction: Candidate was approximately 2.75× faster. Without a page index, Random 0.1% still favored Candidate even though Candidate and Query RowFilter OFF each read exactly 1,040,567,863 bytes. This is consistent with reduced decoding/materialization work, but individual decode costs were not measured directly. The Random 1% result is inconclusive: the direction reversed between JVM runs and Candidate had a wide IQR. It does not establish a 1% switching threshold. In the Sorted 10% cases, approximately 95.8%–98.4% of rows surviving metadata pruning already matched, leaving relatively little additional payload work for RowFilter to avoid. There is also an unresolved control discrepancy: an earlier unfiltered payload query measured Candidate 393.98 ms versus Query RowFilter OFF 455.91 ms, despite identical reader bytes. That control was not rerun in the later per-query-JVM batches. Small differences should therefore be interpreted cautiously. All measured results were checked against Spark, with expected matching counts and payload aggregates also checked for the wide cases. Prior verification of the same libraries covered equality/position deletes, mixed deletes, nullable and unprojected equality keys, and V3 deletion vectors; the later workload runs did not repeat delete verification. These local warm-cache experiments support investigating independent metadata-pruning and query RowFilter controls, with a decision informed by projection cost and selectivity after metadata pruning. They do not support universally disabling query RowFilter or establish an automatic policy. I plan to keep that reader-policy work separate from the decimal pushdown implementation in this PR. </details> -- 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]
