Skip to content

perf: reduce invariant-check code generation - #25073

Open
kumarUjjawal wants to merge 1 commit into
apache:mainfrom
kumarUjjawal:optimize/invariant-check-code-size
Open

perf: reduce invariant-check code generation#25073
kumarUjjawal wants to merge 1 commit into
apache:mainfrom
kumarUjjawal:optimize/invariant-check-code-size

Conversation

@kumarUjjawal

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

check_default_invariants is generic over each ExecutionPlan implementation. The compiler therefore generated many copies of its dynamic-expression and input-distribution validation logic.

These checks depend only on erased values such as the plan name, children, and dynamic expressions. Sharing the heavier validation paths reduces generated code without changing invariant semantics or the public API.

Local release measurements against upstream/main at 92746a993:

Measurement upstream/main This PR Change
Targeted invariant-check LLVM IR lines 68,856 47,082 -31.6%
Total physical-plan LLVM IR lines 2,560,976 2,513,101 -1.87%
Release rlib size 56,223,208 bytes 55,689,856 bytes -0.95%
Linked partial_ordering benchmark binary 5,882,960 bytes 5,882,960 bytes No change

The linked benchmark binary was unchanged because its linker removed unused code. This PR does not claim a measured end-to-end WASM binary reduction.

The added Criterion benchmark showed no runtime regression:

Case upstream/main This PR
Leaf plan 13.016–13.030 ns 12.939–12.951 ns
Four-child plan 150.24–151.02 ns 142.75–145.68 ns

What changes are included in this PR?

  • Move dynamic-expression ID validation behind a non-generic helper.
  • Make input-distribution invariant validation non-generic.
  • Keep the inexpensive per-plan length checks inline.
  • Skip dynamic-expression validation when a plan produces no dynamic expressions.
  • Preserve the public check_default_invariants signature and existing error behavior.
  • Add unit tests for malformed vectors, dynamic-expression IDs, co-partitioning requirements, and trait-object callers.
  • Add a Criterion benchmark for leaf and four-child plans.

What is the testing strategy for this PR?

The new unit tests cover:

  • incorrect invariant-vector lengths and their error messages;
  • missing and duplicate dynamic-expression IDs;
  • malformed and valid co-partitioning requirements;
  • calls through concrete plans and dyn ExecutionPlan.

The following checks pass:

cargo fmt --all

cargo clippy --all-targets --all-features -- -D warnings

RUST_BACKTRACE=1 cargo test --profile ci \
  --exclude datafusion-examples \
  --exclude datafusion-benchmarks \
  --exclude datafusion-cli \
  --workspace --lib --tests --bins \
  --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption

The code-size comparison and runtime measurements use the new invariant_check Criterion benchmark and release builds of datafusion-physical-plan.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Sep 8, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.29545% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.75%. Comparing base (92746a9) to head (b740eab).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/physical-plan/src/execution_plan.rs 98.01% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #25073    +/-   ##
========================================
  Coverage   81.74%   81.75%            
========================================
  Files        1128     1128            
  Lines      416645   416795   +150     
  Branches   416645   416795   +150     
========================================
+ Hits       340580   340741   +161     
+ Misses      55998    55994     -4     
+ Partials    20067    20060     -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

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants