IGNITE-21052 Improved measurement of first query planning time#2960
Merged
xtern merged 10 commits intoapache:mainfrom Dec 22, 2023
Merged
IGNITE-21052 Improved measurement of first query planning time#2960xtern merged 10 commits intoapache:mainfrom
xtern merged 10 commits intoapache:mainfrom
Conversation
xtern
changed the title
xtern
changed the title
| import static java.util.stream.Collectors.toList; | ||
| import static org.apache.ignite.internal.util.IgniteUtils.igniteClassLoader; | ||
|
|
||
| import io.github.classgraph.ClassGraph; |
Contributor
There was a problem hiding this comment.
can we remove the dependency at all?
Contributor
Author
There was a problem hiding this comment.
Thank you. It seems so. The dependency has been removed.
| private RelOptCluster cluster; | ||
|
|
||
| static { | ||
| // Preload some classes to reduce first planning time. |
Contributor
There was a problem hiding this comment.
In the fact visible time of first planning will be the same.
I see tow options here:
- Move loading and initialization classes on earlier stage, during node startup. Due to java could do classpoading in parallel it shouldn't significant increase time of node start, but our very first parsing will be visible faster. For tests which could instantiate some of services himself we could keep initialization in two places.
- Just fix a coomet to show real life )
Contributor
Author
There was a problem hiding this comment.
Thanks, I expanded the comment 😎 .
lowka
approved these changes
Dec 21, 2023
Contributor
|
I have the same concerns as Yuri (point 1 in his comment). But I think it is out of scope of this PR. |
AMashenkov
approved these changes
Dec 22, 2023
ygerzhedovich
approved these changes
Dec 22, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/IGNITE-21052
Issue:
first query planning takes much longer time, mainly due to class loading.
We found that the most time was spent scanning the class path during initialization of
RelMetadataQueryEx.And this is redundant now, since from Calcite 1.28.0 there is no need to register all known RelNode subtypes (see CALCITE-4546 for details).
Also it was found that initialization of
PlannerPhaseenum loads 600+ classes, and this loading affects planning time.So I decided:
RelNodesubtypes registration during initialization ofRelMetadataQueryEx(on TeamCity it took up to 5 seconds)PlannerPhaserelated classes during static initialization of planner (on TeamCity it took up to 2 seconds).After these changes I run unit tests on TC 30+ times without timeout: https://ci.ignite.apache.org/viewType.html?buildTypeId=ApacheIgnite3xGradle_Test_RunUnitTests&branch_ApacheIgnite3xGradle_Test=pull%2F2960&tab=buildTypeStatusDiv
I also analyzed over 10 runs and the (first) planning time now looks more stable, between 1800-2100ms.