RFR (M) JDK-8202605: Standardize on ClassLoaderData::loader_name() throughout the VM to obtain a class loader's name
goetz.lindenmaier at sap.com
Thu Jun 14 12:23:38 UTC 2018
thanks for doing this change!
I appreciate a clear guidance on the naming of classloaders.
Adding a new field to ClassLoader is the best way to
assure this is used widely.
Some comments on the format:
* I would skip the space between the name and the @.
* If two loaders have the same name, it might be helpful
to see the classname. But I guess this will mostly
happen if the loaders are of the same class, too.
(like loading the same library twice. In this case
the id helps).
Please use external_name():
+ _class_loader_name_id = _class_loader_klass->name();
I would rename loader_name() to loader_nameAndId().
Line 400: eventually adapt the comment to use ' ' instead of <>.
I would remove the double quotes from this output.
But you can leave this to a follow up I guess.
Maybe you want to adapt classLoaderStats.cpp:114 to say
'bootstrap' instead of <boot class loader>.
+// Returns the name of this class loader or null if this class loader is not named.
"the name" is ambiguous now. Maybe say "Returns the name _field_ of ..."
I would add to the comment:
// Use ClassLoaderData::loader_name() to obtain this String as a char* for internal use.
Don't you want to put this field into classLoaderData.hpp?
Then you can use it in loader_name(), too.
I think you need to edit the other names, too: "Kevin" --> "'Kevin'" etc.
Or, if you followed my above comment, remove the double quotes altogether.
> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
> Behalf Of Lois Foltan
> Sent: Donnerstag, 14. Juni 2018 00:59
> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> Subject: RFR (M) JDK-8202605: Standardize on
> ClassLoaderData::loader_name() throughout the VM to obtain a class
> loader's name
> Please review this change to standardize on how to obtain a class
> loader's name within the VM. SystemDictionary::loader_name() methods
> have been removed in favor of ClassLoaderData::loader_name().
> Since the loader name is largely used in the VM for display purposes
> (error messages, logging, jcmd, JFR) this change also adopts a new
> format to append to a class loader's name its identityHashCode and if
> the loader has not been explicitly named it's qualified class name is
> used instead.
> 391 /**
> 392 * If the defining loader has a name explicitly set then
> 393 * '<loader-name>' @<id>
> 394 * If the defining loader has no name then
> 395 * <qualified-class-name> @<id>
> 396 * If it's built-in loader then omit `@<id>` as there is only one
> 397 */
> The names for the builtin loaders are 'bootstrap', 'app' and 'platform'.
> open webrev at
> bug link at https://bugs.openjdk.java.net/browse/JDK-8202605
> Testing: hs-tier(1-2), jdk-tier(1-2) complete
> hs-tier(3-5), jdk-tier(3) in progress
More information about the hotspot-dev