-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8351640: Print reason for making method not entrant #23980
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
8351640: Print reason for making method not entrant #23980
Conversation
👋 Welcome back shade! A progress list of the required criteria for merging this PR into |
@shipilev This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 9 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
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.
Good.
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.
Looks good.
Do you mind incorporating log compilation tool support? [1]
diff --git a/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/LogParser.java b/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/LogParser.java
index e1e305abe10..61cbc054200 100644
--- a/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/LogParser.java
+++ b/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/LogParser.java
@@ -1099,6 +1099,10 @@ public void startElement(String uri, String localName, String qname, Attributes
e.setCompileKind(compileKind);
String level = atts.getValue("level");
e.setLevel(level);
+ String reason = atts.getValue("reason");
+ if (reason != null) {
+ e.setReason(reason);
+ }
events.add(e);
} else if (qname.equals("uncommon_trap")) {
String id = atts.getValue("compile_id");
diff --git a/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/MakeNotEntrantEvent.java b/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/MakeNotEntrantEvent.java
index b4015537c74..d230f1b4336 100644
--- a/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/MakeNotEntrantEvent.java
+++ b/src/utils/LogCompilation/src/main/java/com/sun/hotspot/tools/compiler/MakeNotEntrantEvent.java
@@ -47,6 +47,11 @@ class MakeNotEntrantEvent extends BasicLogEvent {
*/
private String level;
+ /**
+ * The reason of invalidation.
+ */
+ private String reason;
+
/**
* The compile kind.
*/
@@ -64,10 +69,14 @@ public NMethod getNMethod() {
public void print(PrintStream stream, boolean printID) {
if (isZombie()) {
- stream.printf("%s make_zombie\n", getId());
+ stream.printf("%s make_zombie", getId());
} else {
- stream.printf("%s make_not_entrant\n", getId());
+ stream.printf("%s make_not_entrant", getId());
+ }
+ if (getReason() != null) {
+ stream.printf(": %s", getReason());
}
+ stream.println();
}
public boolean isZombie() {
@@ -88,7 +97,21 @@ public void setLevel(String level) {
this.level = level;
}
- /**
+ /**
+ * @return the reason
+ */
+ public String getReason() {
+ return reason;
+ }
+
+ /**
+ * @param reason the reason to set
+ */
+ public void setReason(String reason) {
+ this.reason = reason;
+ }
+
+ /**
* @return the compileKind
*/
public String getCompileKind() {
I don't mind, added. Looks like this still works:
|
/contributor add @iwanowww |
@shipilev |
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. Looks good.
Thanks! I'll integrate once GHA clears. |
/integrate |
Going to push as commit 930455b.
Your commit was automatically rebased without conflicts. |
A simple quality of life improvement. We are studying compiler dynamics in Leyden, and it would be convenient to know why the particular methods are marked as not entrant. We just need to pass the extra string argument to
nmethod::make_not_entrant
and print it out.Sample log excerpt for mainline:
You can now clearly see the method lifecycle. 1 second in app lifetime, the method was initially compiled at level 3. Shortly after, it got compiled at level 4, turning level 3 method unused. 4 seconds later, level 4 method encountered uncommon trap, so we are back to level 3. After 1.3 seconds more, the final compilation at level 4 completed, and second level 3 compilation was removed as unused.
Additional testing:
hotspot:tier1
all
Progress
Issue
Reviewers
Contributors
<vlivanov@openjdk.org>
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23980/head:pull/23980
$ git checkout pull/23980
Update a local copy of the PR:
$ git checkout pull/23980
$ git pull https://git.openjdk.org/jdk.git pull/23980/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 23980
View PR using the GUI difftool:
$ git pr show -t 23980
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23980.diff
Using Webrev
Link to Webrev Comment