Clean up node flag tracing - #6846
Merged
Merged
Conversation
jdmpapin
requested review from
0xdaryl,
Leonardo2718,
mstoodle and
vijaysun-omr
as code owners
December 13, 2022 16:45
vijaysun-omr
approved these changes
Jan 16, 2023
Contributor
|
Jenkins build all |
2 similar comments
Contributor
Author
|
Jenkins build all |
Contributor
|
Jenkins build all |
Contributor
Author
|
I can't tell what went wrong in the macOS build ("Build not found"), and it doesn't seem to be restarting when I hit "Re-run." The Windows tests failed because a number of tests didn't run, and in every case it seems to have been because an executable was missing: I can't tell why these files were missing, but I doubt it has to do with node flag tracing. |
jdmpapin
force-pushed
the
node-flag-tracing
branch
from
January 25, 2023 17:48
01ea98f to
13b4d90
Compare
It's very confusing when debugging for a node to have flags that don't appear in the log. Start printing the following flags: - swappedChildren - versionableIfWithMinExpr - versionableIfWithMaxExpr - createdByPRE - referenceIsNonNull
The node flag "print" methods are trivial. They are almost all
one-liners, and their definitions are highly repetitive (not within
definitions, but across them). These methods really only serve as a
noisy and boilerplate-heavy way to specify the flags' names.
Furthermore, each has exactly one use site, so they don't even
meaningfully deduplicate those names in the source code.
Now these methods are inlined into TR_Debug::nodePrintAllFlags(), which
centralizes the choice of names for all flags in trace logs. This most
likely helps the performance of node flag printing on its own, since
previously it was not possible for the build compiler to inline any of
the "print" methods (except for one). Additionally, and in part enabled
by the inlining,
- the repetitive structure is factored out;
- unset flags now skip appending, instead of appending ""; and
- output is generated using appends() instead of appendf("%s", ...).
After inlining, the "print" methods are unused, and they are removed.
This commit does not change the set of flags printed explicitly in
nodePrintAllFlags(), their printed names, or their order. Specifically,
OpenJ9-specific flags are still printed here. Handling of those flags
should be moved to OpenJ9, but that move is not tackled here.
jdmpapin
force-pushed
the
node-flag-tracing
branch
from
January 25, 2023 17:49
13b4d90 to
0feed90
Compare
Contributor
Author
|
I added and removed an empty commit to restart the checks, but it restarted the copyright check as well, so now I've updated the copyright dates. |
Contributor
Author
|
Jenkins build all |
vijaysun-omr
approved these changes
Jan 26, 2023
vijaysun-omr
approved these changes
Jan 26, 2023
Contributor
|
I reviewed that all the queries passed in to the new |
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
2 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.
Please see the individual commit messages.