RFR: 8282172: CompileBroker::log_metaspace_failure is called from non-Java/compiler threads

Aleksey Shipilev shade at openjdk.java.net
Wed Feb 23 07:40:42 UTC 2022


On Tue, 22 Feb 2022 16:59:41 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> No, it could be called from Java execution thread too when profiling is triggered. But yes it was assumed that only Java thread can ask for creation `MethodData`. On other hand `log()` methods accepts `Thread*` parameter and just record it: https://github.com/openjdk/jdk/blob/master/src/hotspot/share/utilities/events.hpp#L183 May be we don't need to cast it to `JavaThread` in this case.

Yes, this would also work:


diff --git a/src/hotspot/share/compiler/compileBroker.cpp b/src/hotspot/share/compiler/compileBroker.cpp
index 1c8656044b5..62c45e6adca 100644
--- a/src/hotspot/share/compiler/compileBroker.cpp
+++ b/src/hotspot/share/compiler/compileBroker.cpp
@@ -227,11 +227,11 @@ class CompilationLog : public StringEventLog {
   void log_metaspace_failure(const char* reason) {
     ResourceMark rm;
     StringLogMessage lm;
     lm.print("%4d   COMPILE PROFILING SKIPPED: %s", -1, reason);
     lm.print("\n");
-    log(JavaThread::current(), "%s", (const char*)lm);
+    log(Thread::current(), "%s", (const char*)lm);
   }
 };
 
 static CompilationLog* _compilation_log = NULL;


...but it is odd to: a) have the rest of `CompilationLog` accept `JavaThread` only; b) having `COMPILE SKIPPED` printed when we are not dealing with any compiler/Java thread, for example from VMThread / JVMTI code like on the path in the PR body.


void CompileBroker::log_metaspace_failure() {
  const char* message = "some methods may not be compiled because metaspace "
                        "is out of memory";
  if (_compilation_log != NULL) {
    _compilation_log->log_metaspace_failure(message);
  }
  if (PrintCompilation) {
    tty->print_cr("COMPILE PROFILING SKIPPED: %s", message); // <--- from here
  }
}


So I still believe filtering non-Java/compiler threads at `CompileBroker::log_metaspace_failure()` is the proper fix.

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

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


More information about the hotspot-compiler-dev mailing list