8154791: Xlog classload too redundant msgs info/debug
authorrprotacio
Mon, 08 May 2017 09:45:24 -0400
changeset 46444 677be3444372
parent 46443 cdb638b5ec53
child 46445 825b002e05ae
8154791: Xlog classload too redundant msgs info/debug Summary: Removed redundant information from class+load UL messages, ensured side-by-side printing of different levels for same class, cleaned up code Reviewed-by: iklam, dholmes
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/systemDictionary.cpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/test/runtime/modules/PatchModule/PatchModuleTraceCL.java
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Mon May 08 07:16:10 2017 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Mon May 08 09:45:24 2017 -0400
@@ -5482,14 +5482,7 @@
     if (log_is_enabled(Info, class, load)) {
       ResourceMark rm;
       const char* module_name = (module_entry->name() == NULL) ? UNNAMED_MODULE : module_entry->name()->as_C_string();
-
-      if (log_is_enabled(Info, class, load)) {
-        ik->print_loading_log(LogLevel::Info, _loader_data, module_name, _stream);
-      }
-      // No 'else' here as logging levels are not mutually exclusive
-      if (log_is_enabled(Debug, class, load)) {
-        ik->print_loading_log(LogLevel::Debug, _loader_data, module_name, _stream);
-      }
+      ik->print_class_load_logging(_loader_data, module_name, _stream);
     }
 
     if (log_is_enabled(Debug, class, resolve))  {
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp	Mon May 08 07:16:10 2017 -0400
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp	Mon May 08 09:45:24 2017 -0400
@@ -1384,14 +1384,7 @@
       ik->restore_unshareable_info(loader_data, protection_domain, CHECK_NULL);
     }
 
-    if (log_is_enabled(Info, class, load)) {
-      ik->print_loading_log(LogLevel::Info, loader_data, NULL, NULL);
-    }
-    // No 'else' here as logging levels are not mutually exclusive
-
-    if (log_is_enabled(Debug, class, load)) {
-      ik->print_loading_log(LogLevel::Debug, loader_data, NULL, NULL);
-    }
+    ik->print_class_load_logging(loader_data, NULL, NULL);
 
     // For boot loader, ensure that GetSystemPackage knows that a class in this
     // package was loaded.
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon May 08 07:16:10 2017 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon May 08 09:45:24 2017 -0400
@@ -41,6 +41,7 @@
 #include "interpreter/rewriter.hpp"
 #include "jvmtifiles/jvmti.h"
 #include "logging/log.hpp"
+#include "logging/logMessage.hpp"
 #include "memory/heapInspection.hpp"
 #include "memory/iterator.inline.hpp"
 #include "memory/metadataFactory.hpp"
@@ -72,7 +73,6 @@
 #include "utilities/dtrace.hpp"
 #include "utilities/macros.hpp"
 #include "utilities/stringUtils.hpp"
-#include "logging/log.hpp"
 #ifdef COMPILER1
 #include "c1/c1_Compiler.hpp"
 #endif
@@ -3058,37 +3058,31 @@
   return external_name();
 }
 
-void InstanceKlass::print_loading_log(LogLevel::type type,
-                                      ClassLoaderData* loader_data,
-                                      const char* module_name,
-                                      const ClassFileStream* cfs) const {
+void InstanceKlass::print_class_load_logging(ClassLoaderData* loader_data,
+                                             const char* module_name,
+                                             const ClassFileStream* cfs) const {
+  if (!log_is_enabled(Info, class, load)) {
+    return;
+  }
+
   ResourceMark rm;
-  outputStream* log;
-
-  assert(type == LogLevel::Info || type == LogLevel::Debug, "sanity");
-
-  if (type == LogLevel::Info) {
-    log = Log(class, load)::info_stream();
-  } else {
-    assert(type == LogLevel::Debug,
-           "print_loading_log supports only Debug and Info levels");
-    log = Log(class, load)::debug_stream();
-  }
+  LogMessage(class, load) msg;
+  stringStream info_stream;
 
   // Name and class hierarchy info
-  log->print("%s", external_name());
+  info_stream.print("%s", external_name());
 
   // Source
   if (cfs != NULL) {
     if (cfs->source() != NULL) {
       if (module_name != NULL) {
         if (ClassLoader::is_jrt(cfs->source())) {
-          log->print(" source: jrt:/%s", module_name);
+          info_stream.print(" source: jrt:/%s", module_name);
         } else {
-          log->print(" source: %s", cfs->source());
+          info_stream.print(" source: %s", cfs->source());
         }
       } else {
-        log->print(" source: %s", cfs->source());
+        info_stream.print(" source: %s", cfs->source());
       }
     } else if (loader_data == ClassLoaderData::the_null_class_loader_data()) {
       Thread* THREAD = Thread::current();
@@ -3098,46 +3092,52 @@
                 : NULL;
       // caller can be NULL, for example, during a JVMTI VM_Init hook
       if (caller != NULL) {
-        log->print(" source: instance of %s", caller->external_name());
+        info_stream.print(" source: instance of %s", caller->external_name());
       } else {
         // source is unknown
       }
     } else {
       oop class_loader = loader_data->class_loader();
-      log->print(" source: %s", class_loader->klass()->external_name());
+      info_stream.print(" source: %s", class_loader->klass()->external_name());
     }
   } else {
-    log->print(" source: shared objects file");
+    info_stream.print(" source: shared objects file");
   }
 
-  if (type == LogLevel::Debug) {
+  msg.info("%s", info_stream.as_string());
+
+  if (log_is_enabled(Debug, class, load)) {
+    stringStream debug_stream;
+
     // Class hierarchy info
-    log->print(" klass: " INTPTR_FORMAT " super: " INTPTR_FORMAT,
-               p2i(this),  p2i(superklass()));
-
+    debug_stream.print(" klass: " INTPTR_FORMAT " super: " INTPTR_FORMAT,
+                       p2i(this),  p2i(superklass()));
+
+    // Interfaces
     if (local_interfaces() != NULL && local_interfaces()->length() > 0) {
-      log->print(" interfaces:");
+      debug_stream.print(" interfaces:");
       int length = local_interfaces()->length();
       for (int i = 0; i < length; i++) {
-        log->print(" " INTPTR_FORMAT,
-                   p2i(InstanceKlass::cast(local_interfaces()->at(i))));
+        debug_stream.print(" " INTPTR_FORMAT,
+                           p2i(InstanceKlass::cast(local_interfaces()->at(i))));
       }
     }
 
     // Class loader
-    log->print(" loader: [");
-    loader_data->print_value_on(log);
-    log->print("]");
+    debug_stream.print(" loader: [");
+    loader_data->print_value_on(&debug_stream);
+    debug_stream.print("]");
 
     // Classfile checksum
     if (cfs) {
-      log->print(" bytes: %d checksum: %08x",
-                 cfs->length(),
-                 ClassLoader::crc32(0, (const char*)cfs->buffer(),
-                 cfs->length()));
+      debug_stream.print(" bytes: %d checksum: %08x",
+                         cfs->length(),
+                         ClassLoader::crc32(0, (const char*)cfs->buffer(),
+                         cfs->length()));
     }
+
+    msg.debug("%s", debug_stream.as_string());
   }
-  log->cr();
 }
 
 #if INCLUDE_SERVICES
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Mon May 08 07:16:10 2017 -0400
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Mon May 08 09:45:24 2017 -0400
@@ -30,7 +30,6 @@
 #include "classfile/moduleEntry.hpp"
 #include "classfile/packageEntry.hpp"
 #include "gc/shared/specialized_oop_closures.hpp"
-#include "logging/logLevel.hpp"
 #include "memory/referenceType.hpp"
 #include "oops/annotations.hpp"
 #include "oops/constMethod.hpp"
@@ -1369,8 +1368,9 @@
   void oop_verify_on(oop obj, outputStream* st);
 
   // Logging
-  void print_loading_log(LogLevel::type type, ClassLoaderData* loader_data,
-                         const char* module_name, const ClassFileStream* cfs) const;
+  void print_class_load_logging(ClassLoaderData* loader_data,
+                                const char* module_name,
+                                const ClassFileStream* cfs) const;
 };
 
 // for adding methods
--- a/hotspot/test/runtime/modules/PatchModule/PatchModuleTraceCL.java	Mon May 08 07:16:10 2017 -0400
+++ b/hotspot/test/runtime/modules/PatchModule/PatchModuleTraceCL.java	Mon May 08 09:45:24 2017 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,7 +24,7 @@
 /*
  * @test
  * @bug 8069469
- * @summary Make sure -Xlog:classload=info works properly with "modules" jimage,
+ * @summary Make sure -Xlog:class+load=info works properly with "modules" jimage,
             --patch-module, and with -Xbootclasspath/a
  * @modules java.base/jdk.internal.misc
  * @library /test/lib
@@ -47,7 +47,7 @@
                         "    } "                                    +
                         "}";
 
-        // Test -Xlog:classload=info output for --patch-module
+        // Test -Xlog:class+load=info output for --patch-module
         ClassFileInstaller.writeClassToDisk("javax/naming/spi/NamingManager",
              InMemoryJavaCompiler.compile("javax.naming.spi.NamingManager", source, "-Xmodule:java.naming"),
              "mods/java.naming");
@@ -63,7 +63,7 @@
         // -cp case.
         output.shouldContain("[class,load] PatchModuleMain source: file");
 
-        // Test -Xlog:classload=info output for -Xbootclasspath/a
+        // Test -Xlog:class+load=info output for -Xbootclasspath/a
         source = "package PatchModuleTraceCL_pkg; "                 +
                  "public class ItIsI { "                          +
                  "    static { "                                  +