RFR: 8316229: Enhance class initialization logging

Aleksey Shipilev shade at openjdk.org
Tue Sep 19 07:53:40 UTC 2023


On Tue, 19 Sep 2023 06:33:02 GMT, David Holmes <dholmes at openjdk.org> wrote:

> This change adds some additional debug logging to the class linking and initialization process. It was useful in diagnosing the deadlock described by:
> 
> https://bugs.openjdk.org/browse/JDK-8316469
> 
> See the example output in JBS issue.
> 
> The changes are mostly uncontroversial but I needed to expose a way to access the init thread's name, which can't use the regular `name()` method as the safety check doesn't recognise the calling context as being safe.
> 
> Testing:
>  - manual examination of logging output
>  - ran the only test that enables class+init logging: runtime/logging/ClassInitializationTest.java (no change as expected)
>  - tiers1-3 sanity
> 
> Thanks.

This looks good, I have only stylistic questions/suggestions.

src/hotspot/share/oops/instanceKlass.cpp line 773:

> 771:   MonitorLocker ml(current, _init_monitor);
> 772: 
> 773:   bool debug_logging_enabled = log_is_enabled(Debug, class, init);

Here and later: Do we need to peel off the `log_is_enabled` check into a separate variable? We don't do it in most (all?) of our places.

src/hotspot/share/oops/instanceKlass.cpp line 779:

> 777:     if (debug_logging_enabled) {
> 778:       ResourceMark rm(current);
> 779:       log_debug(class, init)("Thread %s waiting for linking of %s by thread %s",

Here and later: Should thread names be in quotes? This would match other places, e.g. error handler, jstack output, etc.

src/hotspot/share/oops/instanceKlass.hpp line 502:

> 500:   // We can safely access the name as long as we hold the _init_monitor.
> 501:   const char* init_thread_name() {
> 502:     assert(_init_monitor->owned_by_self(), " Must hold _init_monitor here");

Suggestion:

    assert(_init_monitor->owned_by_self(), "Must hold _init_monitor here");

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

PR Review: https://git.openjdk.org/jdk/pull/15809#pullrequestreview-1632632886
PR Review Comment: https://git.openjdk.org/jdk/pull/15809#discussion_r1329714158
PR Review Comment: https://git.openjdk.org/jdk/pull/15809#discussion_r1329714897
PR Review Comment: https://git.openjdk.org/jdk/pull/15809#discussion_r1329716244


More information about the hotspot-dev mailing list