-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[opt](memo) reuse GroupPlan in Group to reduce memory usage #59877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 31103 ms |
TPC-DS: Total hot run time: 173727 ms |
ClickBench: Total hot run time: 26.72 s |
c97673b to
7136e6b
Compare
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
yujun777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
TPC-H: Total hot run time: 32226 ms |
TPC-DS: Total hot run time: 173863 ms |
ClickBench: Total hot run time: 27.12 s |
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
FE Regression Coverage ReportIncrement line coverage |
7136e6b to
3e936dd
Compare
|
run buildall |
TPC-H: Total hot run time: 31764 ms |
TPC-DS: Total hot run time: 176562 ms |
ClickBench: Total hot run time: 26.87 s |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by at least one committer and no changes requested. |
FE Regression Coverage ReportIncrement line coverage |
What problem does this PR solve?
Problem Summary:
This pull request refactors how
GroupPlaninstances are created and accessed throughout the codebase. Instead of creating newGroupPlanobjects from aGroup, the code now consistently uses a singleGroupPlaninstance perGroupvia a newgetGroupPlan()method. This improves memory efficiency, code clarity, and ensures consistency across the planner and matcher logic.Core API and Data Model Changes
GroupPlanfield to theGroupclass and initialized it in constructors; exposed it via a newgetGroupPlan()method. This ensures eachGrouphas a unique, reusableGroupPlaninstance. [1] [2] [3] [4]Refactoring: Use of GroupPlan
GroupPlanwas constructed from aGroupto instead use thegetGroupPlan()method, including planner logic, memoization, and pattern matching. This change appears in files such asHyperGraph.java,PlanReceiver.java,Memo.java,GroupExpressionMatching.java,GroupMatching.java,DistributionSpec.java, andOrderSpec.java. [1] [2] [3] [4] [5] [6] [7] [8] [9]These changes collectively improve the consistency and efficiency of plan representation in the query optimizer.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)