-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Make rule initialization static #125590
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
Make rule initialization static #125590
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
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.
initialization is hilighted with pink and accounts for ~1.59% of cpu
Of what? I guess this isn't the entire planning, right? It'd be interesting to see what difference this PR makes too.
But in any case, the change makes sense to me.
Correct, 1.59 of parent that seems to be planning |
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.
Thanks @idegtiarenko , this is a nice simplification and optimization.
We have some slightly stateful renaming mechanisms in some optimizer rules (whenever temporary names need to be created); I double checked and they either rely on local variables (defined in the scope of a method) or synthetic static variables, so all looks good there.
(cherry picked from commit 2ce2fda)
LogicalPlanOptimizer & LocalLogicalPlanOptimizer were surprisingly visible in the profile output around initializing their long list of rules for each request (mostly spending time around creating loggers).
They do not have a state and could be shared across sessions reducing per session initialization time and amount of objects created.
(^^ initialization is hilighted with pink and accounts for ~1.59% of cpu)