devanbenz commented on PR #27126:
URL: https://github.com/apache/datafusion/pull/27126#issuecomment-6689718905

   I am unable to run benchmarks, but, could we get them ran against tpcds with 
and without pushdown and tpch10 without pushdown too in the PR?
   
   I ran through some more benchmarks locally and noticed some regressions: 
   
   | Benchmark | `main` (70f2ed01b) | PR (a2c3adb04) | Change | Slower / faster 
(>5%) |
   |---|---:|---:|---:|---:|
   | TPC-DS SF1 | 7,857 ms | 8,663 ms | +10.3% | 49 / 12 |
   | TPC-DS SF1, `--simulate-latency` | 54,417 ms | 75,615 ms | +39.0% | 73 / 
13 |
   | TPC-H SF10 | 8,268 ms | 8,384 ms | +1.4% | 1 / 1 |
   | Synthetic: 3 CollectLeft aggregate builds, `target_partitions=4` | 573 ms 
| 647 ms | +12.9% | n/a |
   | Synthetic: same, `target_partitions=20` | 246 ms | 263 ms | +6.9% | n/a |
   
   | TPC-DS query | `main` | PR | Change | `main` (latency) | PR (latency) | 
Change (latency) |
   |---|---:|---:|---:|---:|---:|---:|
   | q88 | 50.2 ms | 158.4 ms | 3.16x | 367 ms | 3214 ms | 8.76x |
   | q28 | 57.7 ms | 157.4 ms | 2.73x | 338 ms | 1045 ms | 3.09x |
   | q46 | 9.7 ms | 12.6 ms | 1.29x | 123 ms | 347 ms | 2.81x |
   | q89 | 30.4 ms | 32.2 ms | 1.06x | 221 ms | 480 ms | 2.17x |
   | q73 | 9.4 ms | 8.8 ms | 0.93x | 59 ms | 127 ms | 2.17x |
   | q38 | 50.6 ms | 58.9 ms | 1.16x | 423 ms | 907 ms | 2.14x |
   | q81 | 24.1 ms | 33.5 ms | 1.39x | 318 ms | 672 ms | 2.12x |
   | q84 | 24.1 ms | 30.8 ms | 1.28x | 226 ms | 438 ms | 1.94x |
   | q19 | 35.3 ms | 35.6 ms | 1.01x | 296 ms | 569 ms | 1.92x |
   | q74 | 139.9 ms | 157.5 ms | 1.13x | 856 ms | 1628 ms | 1.90x | 
   
   I think this change makes it so the build sides that used to run in parallel 
run sequentially now? Very much restricting parallelism. It helps when there is 
a dynamic filter to prune data but for anything else it limits parallelism.
   
   Something akin to this:
   
    ```
   main (eager): all build sides start at execute()
     build A  ████████
     build B  ██████
     build C  ███████
     probe            ████
              └─ total ≈ longest build + probe
   
   PR (lazy): each build starts only when its join is polled
     build A  ████████
     build B          ██████
     build C                ███████
     probe                         ████
              └─ 
   ``` 
   
   (Thanks claude for taking my word vomit and turning it in to a nice little 
diagram :laughing:) 


-- 
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]

Reply via email to