RFR: 8338939: Simplify processing of hidden class names
Greetings, The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another. The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602". With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000". An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600". It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/"). Testing: jdk_jfr Thanks Markus ------------- Commit messages: - external_name - 8338939 Changes: https://git.openjdk.org/jdk/pull/20708/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20708&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338939 Stats: 58 lines in 2 files changed: 2 ins; 17 del; 39 mod Patch: https://git.openjdk.org/jdk/pull/20708.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20708/head:pull/20708 PR: https://git.openjdk.org/jdk/pull/20708
On Sun, 25 Aug 2024 18:29:47 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another.
The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602".
With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000".
An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600".
It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/").
Testing: jdk_jfr
Thanks Markus
src/hotspot/share/jfr/support/jfrSymbolTable.cpp line 259:
257: const size_t len = static_cast<size_t>(name->utf8_length()); 258: char* hidden_klass_name = NEW_RESOURCE_ARRAY(char, len + 1); 259: strncpy(hidden_klass_name, name->as_klass_external_name(), len + 1);
Can't you use the `k->external_name()` which performs this plus to slash replacement already? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20708#discussion_r1730424838
On Sun, 25 Aug 2024 20:23:15 GMT, Chen Liang <liach@openjdk.org> wrote:
Greetings,
The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another.
The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602".
With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000".
An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600".
It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/").
Testing: jdk_jfr
Thanks Markus
src/hotspot/share/jfr/support/jfrSymbolTable.cpp line 259:
257: const size_t len = static_cast<size_t>(name->utf8_length()); 258: char* hidden_klass_name = NEW_RESOURCE_ARRAY(char, len + 1); 259: strncpy(hidden_klass_name, name->as_klass_external_name(), len + 1);
Can't you use the `k->external_name()` which performs this plus to slash replacement already?
Good point, turns out this support already exists. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20708#discussion_r1730985016
Greetings,
The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another.
The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602".
With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000".
An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600".
It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/").
Testing: jdk_jfr
Thanks Markus
Markus Grönlund has updated the pull request incrementally with two additional commits since the last revision: - cleanup - use existing k->external_name ------------- Changes: - all: https://git.openjdk.org/jdk/pull/20708/files - new: https://git.openjdk.org/jdk/pull/20708/files/805d78d5..cb10d174 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20708&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20708&range=00-01 Stats: 25 lines in 1 file changed: 0 ins; 23 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/20708.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20708/head:pull/20708 PR: https://git.openjdk.org/jdk/pull/20708
On Mon, 26 Aug 2024 10:02:32 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another.
The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602".
With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000".
An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600".
It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/").
Testing: jdk_jfr
Thanks Markus
Markus Grönlund has updated the pull request incrementally with two additional commits since the last revision:
- cleanup - use existing k->external_name
Marked as reviewed by egahlin (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/20708#pullrequestreview-2263568386
On Sun, 25 Aug 2024 18:29:47 GMT, Markus Grönlund <mgronlun@openjdk.org> wrote:
Greetings,
The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another.
The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602".
With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000".
An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600".
It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/").
Testing: jdk_jfr
Thanks Markus
This pull request has now been integrated. Changeset: 0f667103 Author: Markus Grönlund <mgronlun@openjdk.org> URL: https://git.openjdk.org/jdk/commit/0f667103db7842fe9d3399f56baee0a5def4529e Stats: 59 lines in 2 files changed: 2 ins; 40 del; 17 mod 8338939: Simplify processing of hidden class names Reviewed-by: egahlin ------------- PR: https://git.openjdk.org/jdk/pull/20708
participants (3)
-
Chen Liang
-
Erik Gahlin
-
Markus Grönlund