Skip to content

Conversation

ovr
Copy link
Member

@ovr ovr commented Oct 13, 2025

Before optimizations

Dev Build (Debug, Unoptimized)

- Total allocations: 361
- Total stack size: 19,818 bytes (19.35 KB)

Release Build (Optimized)
- Total allocations: 187
- Total stack size: 11,429 bytes (11.16 KB)

Key Findings

  1. Largest allocations are Expr types (144 bytes each):
    - Dev: 98 allocations = 14,112 bytes
    - Release: 52 allocations = 7,488 bytes
    - This is the datafusion::logical_expr::Expr enum
  2. Debug builds have significant overhead:
    - 102 small allocations (8 bytes) reduced to 26 in release
    - 12 tiny allocations (1 byte) completely eliminated

After using Result<Box> as return type

Dev Build:
- Before: 19,818 bytes (19.35 KB)
- After: 12,572 bytes (12.28 KB)
- Improvement: 7,246 bytes saved (36.6% reduction)

Release Build:
- Before: 11,429 bytes (11.16 KB)
- After: 8,206 bytes (8.01 KB)
- Improvement: 3,223 bytes saved (28.2% reduction)

After using Expr as rt for grouping_set_normalize

Dev: -3.0% | Release: -2.6%

Dev Build:
- Before: 12,572 bytes (12.28 KB)
- After: 12,195 bytes (11.91 KB)

Release Build:
- Before: 8,206 bytes (8.01 KB)
- After: 7,990 bytes (7.80 KB)

Optimize binary_expr_normalize

Dev Build:
- Before: 5,054 bytes (55 allocations)
- After: 3,918 bytes (49 allocations)

Release Build:
- binary_expr_normalize: 3,918 bytes (3.83 KB)

Summary

Stage Description Stack Size Reduction binary_expr
1 Original Result<Expr> 11,429 bytes baseline -
2 Result<Box<Expr>> 8,206 bytes -28.2% -
3 grouping_set_normalize opt 7,990 bytes -30.1% 5,054 bytes
4 binary_expr optimize 7,990 bytes -30.1% 3,918 bytes

@ovr ovr force-pushed the fix/plan-normalize-stack-overflow branch from 5526d06 to 3094cd9 Compare October 13, 2025 13:44
@github-actions github-actions bot added the rust Pull requests that update Rust code label Oct 13, 2025
@ovr ovr force-pushed the fix/plan-normalize-stack-overflow branch from 0afecec to 341185e Compare October 13, 2025 13:58
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 91.31944% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.15%. Comparing base (070e6e0) to head (e45516f).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
...src/compile/engine/df/optimizers/plan_normalize.rs 90.31% 25 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #10055       +/-   ##
===========================================
+ Coverage   48.81%   77.15%   +28.33%     
===========================================
  Files         196      440      +244     
  Lines       15787    89270    +73483     
  Branches     3099     3099               
===========================================
+ Hits         7706    68873    +61167     
- Misses       7692    20008    +12316     
  Partials      389      389               
Flag Coverage Δ
cube-backend 48.81% <ø> (ø)
cubesql 83.23% <91.31%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@ovr ovr force-pushed the fix/plan-normalize-stack-overflow branch from 83ea3b7 to f47a892 Compare October 13, 2025 14:35
@ovr ovr force-pushed the fix/plan-normalize-stack-overflow branch from 1e219cc to c738596 Compare October 14, 2025 10:37
@ovr ovr changed the title fix(cubesql): PlanNormalize - stack overflow feat(cubesql): PlanNormalize - reduce stack allocations Oct 14, 2025
@ovr ovr marked this pull request as ready for review October 14, 2025 12:13
@ovr ovr requested a review from a team as a code owner October 14, 2025 12:13
@ovr ovr merged commit 619343d into master Oct 14, 2025
83 of 86 checks passed
@ovr ovr deleted the fix/plan-normalize-stack-overflow branch October 14, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants