RFR (M) JDK-8202605: Standardize on ClassLoaderData::loader_name() throughout the VM to obtain a class loader's name

[Lindenmaier, Goetz]

Hi Lois,

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).

In detail:

classLoaderData.cpp:
  Please use external_name():
  +  _class_loader_name_id = _class_loader_klass->name();

  I would rename loader_name() to loader_nameAndId().


classLoaderData.hpp:
  Line 400: eventually adapt the comment to use ' ' instead of <>.

classLoaderHierarchyDCmd.cpp:
  I would remove the double quotes from this output.
  But you can leave this to a follow up I guess.

classLoaderStats.cpp:
  Maybe you want to adapt classLoaderStats.cpp:114 to say
  'bootstrap' instead of <boot class loader>.

javaClasses.cpp:
  +// 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 ..."

  +java_lang_ClassLoader::nameAndId(oop loader)
  I would add to the comment:
  // Use ClassLoaderData::loader_name() to obtain this String as a char* for internal use.


jfrTypeSetUtils.hpp:
  Don't you want to put this field into classLoaderData.hpp?
  Then you can use it in loader_name(), too.

ClassLoaderHierarchyTest.java:
  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.

Best regards,
  Goetz.


> -----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
> instance.
> 397 */
> 
> The names for the builtin loaders are 'bootstrap', 'app' and 'platform'.
> 
> open webrev at
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8202605/webrev/
> 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
> 
> Thanks,
> Lois