physical plan: add reset_plan_states , plan re-use benchmark
#19806
Conversation
askalt
force-pushed
the
askalt/reuse_plan_bench
branch
from
0ee1d01 to
daadb0c
Compare
askalt
force-pushed
the
askalt/reuse_plan_bench
branch
from
daadb0c to
e28f292
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you for this PR @askalt
I think it is great, though I have a suggestion about how to make it more concise and easy to maintain.
Also, this is a benchmark for reset_plan_states what do you think about renaming the file datafusion/physical-plan/benches/reset_plan_states.rs so the benchmark is easier to find?
| /// Returns a typical plan for the query like: | ||
| /// | ||
| /// ```sql | ||
| /// SELECT aggr1(col1) as aggr1, aggr2(col2) as aggr2 FROM t |
There was a problem hiding this comment.
I think it would be more future proof if you actually crated the plan from SQL rather than building up the plan manually.
That way if the way SQL is planned is changed, then the benchmark will automatically update too and I think the benchmark would be substantially shorter and easier to understand
There was a problem hiding this comment.
Done. Benchmark is moved to core to be able to use physical planner.
askalt
force-pushed
the
askalt/reuse_plan_bench
branch
from
e28f292 to
2506a7b
Compare
This patch adds a benchmark which is intended to measure overhead of actions, required to perform making an independent instance of the execution plan to re-execute it, avoiding re-planning stage. There are several typical plans that are tested, covering projection, aggregation, filtration, re-partition. Also, the function `reset_plan_states(...)` is publically exported.
askalt
force-pushed
the
askalt/reuse_plan_bench
branch
from
2506a7b to
0e56507
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
This is great. I profiled it with samply and indeed a huge amount of time is spent recomputing the properties
alamb
changed the title
reset_plan_states , plan re-use benchmark
|
Thank you @askalt |
2 participants
Which issue does this PR close?
PlanPropertiesredundently #19796What changes are included in this PR?
This patch adds a benchmark which is intended to measure overhead of actions, required to perform making an independent instance of the execution plan to re-execute it, avoiding re-planning stage. There are several typical plans that are tested, covering projection, aggregation, filtration, re-partition.
Are there any user-facing changes?
The function
reset_plan_states(...)is publically exported.