RFR: 8275775: VM.metaspace prints flag 'f' for classes that overrided finalize()

[David Holmes]

On Fri, 22 Oct 2021 06:57:29 GMT, Yi Yang <yyang at openjdk.org> wrote:

> Some customers want to observe which loaded classes have overridden the finalize() method. I found that VM.metaspace can output detailed classes. It seems feasible to add 'f' flag to it. With this patch, I found that ZipFileSystem left a finalize after applying this patch, which was obsolete 5 years ago, maybe we should remove it?

Hi Yi,

I think flagging that the class has a finalizer is potentially useful, but the way this is expressed in not quite right. `has_finalizer()` will be true if the class has a non-trivial finalize() method that will be invoked, either because the class defines it or the class inherits it. To simply refer to this as "overrides" (Note NOT overridded) is inaccurate as you could actually have a class that overrides finalize() with an empty implementation that replaces a non-trivial super-class finalize() method. While this is a questionable thing to do, such a class will not be marked as overriding finalize() even though it has in an important way.

I think the flag should be described as showing "has a non-trivial finalize() method".

Thanks,
David

src/hotspot/share/memory/metaspace/printMetaspaceInfoKlassClosure.cpp line 46:

> 44:   // Print a 's' for shared classes
> 45:   _out->put(k->is_shared() ? 's' : ' ');
> 46:   // Print 's' for classes that overrided finalize() method

s/'s'/'f'/

s/overrided/override/

src/hotspot/share/memory/metaspace/printMetaspaceInfoKlassClosure.cpp line 47:

> 45:   _out->put(k->is_shared() ? 's' : ' ');
> 46:   // Print 's' for classes that overrided finalize() method
> 47:   _out->put(k->has_finalizer() ?'f' : ' ');

Nit: need space after ?

src/hotspot/share/memory/metaspace/printMetaspaceInfoKlassClosure.cpp line 50:

> 48: 
> 49:   ResourceMark rm;
> 50:   _out->print("  %s (" INTPTR_FORMAT ")", k->external_name(), p2i(k));

Why do we need the address printed?

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6075