Skip to content

perf: share one StatisticsContext across all physical optimizer rules - #26094

Draft
asolimando wants to merge 1 commit into
apache:mainfrom
asolimando:asolimando/query-lifetime-stats-cache
Draft

asolimando wants to merge 1 commit into
apache:mainfrom
asolimando:asolimando/query-lifetime-stats-cache

Conversation

@asolimando

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

Each physical optimizer rule that reads statistics creates its own StatisticsContext, so the statistics of the same plan nodes are computed again in every rule. JoinSelection creates a new context for every get_stats call, so it has no caching at all, even within its own pass.

#25098 showed the gain of sharing one context inside EnsureDistribution. #25929 made cache entries keep the plan node they were computed for, so a context can now be shared safely across plan rewrites. This PR uses that to share one StatisticsContext across all the rules of one optimize_physical_plan call.

On TPC-DS (98 queries) and TPC-H (21 queries), sf1 Parquet, planned with a local harness (not part of this PR):

main this PR
Statistics cache misses (statistics computed from scratch) 46,022 25,428 (-45%)
Physical planning time, sum over all queries 294.8 / 293.5 ms 274.3 / 279.7 ms (about -6%)
Physical plans identical for all 119 queries

The largest gain is TPC-DS q64: cache misses go from 3,785 to 531 and planning time drops by about 21%.

What changes are included in this PR?

  • StatisticsContext stores its cache in a parking_lot::Mutex instead of Rc<RefCell>, so it is Send + Sync. This is needed because PhysicalOptimizerContext: Send + Sync. The lock is held only for single map lookups and inserts, never across the recursive walk. The compute_statistics benchmark shows no difference from main (all cases within ±4%, in both directions).
  • New PhysicalOptimizerContext::statistics_context(), which returns None by default. DefaultPhysicalPlanner creates one context per optimize_physical_plan call, built from the session's statistics registry, and returns it to every rule.
  • ConfigOnlyContext also owns a StatisticsContext, so a rule called through optimize() shares one cache for its whole pass.
  • JoinSelection, EnsureRequirements (including PlanSize::from_plan), AggregateStatistics and LimitPushdown use the shared context. When a context does not share one (for example, a context received through FFI), the rules create a new context from the statistics registry, as they did before.
  • pushdown_limit_helper keeps its signature and calls the new pushdown_limit_helper_with_stats, which takes a &StatisticsContext. This is the same pattern as ensure_distribution_with_stats.

What is the testing strategy for this PR?

  • New optimizer_rules_share_statistics_context test in physical_planner.rs: two rules compute the root statistics through the shared context, and the second one gets the Arc cached by the first.
  • Existing tests cover the rewired rules. All sqllogictests pass with no plan changes.

Are there any user-facing changes?

No breaking changes.

  • New default method PhysicalOptimizerContext::statistics_context().
  • New public function pushdown_limit_helper_with_stats.
  • StatisticsContext is now Send + Sync.
  • AggregateStatistics, LimitPushdown and PlanSize::from_plan now consult the session's statistics providers, as JoinSelection and EnsureRequirements already do. Without registered providers (the default), their results are unchanged.

Disclaimer: I used AI to assist in the code generation, I have manually reviewed the output and it matches my intention and understanding.

Create one StatisticsContext per optimize_physical_plan call and expose it
through the new PhysicalOptimizerContext::statistics_context, so statistics
computed by one rule are reused by later rules. JoinSelection,
EnsureRequirements, AggregateStatistics and LimitPushdown use it.

StatisticsContext stores its cache in a parking_lot::Mutex so it is
Send + Sync, as PhysicalOptimizerContext requires.
@github-actions github-actions Bot added optimizer Optimizer rules core Core DataFusion crate physical-plan Changes to the physical-plan crate labels Oct 6, 2026
@asolimando

Copy link
Copy Markdown
Member Author

@kosiew @zhuqi-lucas, could you run run benchmark sql_planner as I don't rights myself?

I figure you'd be interested in this PR as it builds on #25929, and it's the query-lifetime follow-up of what #25098 did for EnsureDistribution.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.71739% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.72%. Comparing base (db83fcc) to head (3179bd9).

Files with missing lines Patch % Lines
datafusion/core/src/physical_planner.rs 77.19% 10 Missing and 3 partials ⚠️
...atafusion/physical-optimizer/src/limit_pushdown.rs 59.37% 8 Missing and 5 partials ⚠️
...er/src/ensure_requirements/enforce_distribution.rs 83.78% 5 Missing and 1 partial ⚠️
...atafusion/physical-optimizer/src/join_selection.rs 58.33% 0 Missing and 5 partials ⚠️
datafusion/physical-plan/src/statistics.rs 66.66% 2 Missing ⚠️
.../physical-optimizer/src/ensure_requirements/mod.rs 85.71% 0 Missing and 1 partial ⚠️
datafusion/physical-optimizer/src/optimizer.rs 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26094      +/-   ##
==========================================
- Coverage   82.72%   82.72%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      448179   448289     +110     
  Branches   448179   448289     +110     
==========================================
+ Hits       370754   370828      +74     
- Misses      54895    54924      +29     
- Partials    22530    22537       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate optimizer Optimizer rules physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants