8151438: UL instantiates duplicate tag sets
authormlarsson
Tue, 29 Mar 2016 09:36:43 +0200
changeset 37200 e601cf49b2d8
parent 37199 74703ea13069
child 37201 928cf689af1a
8151438: UL instantiates duplicate tag sets Reviewed-by: brutisso, stefank
hotspot/src/share/vm/gc/serial/defNewGeneration.cpp
hotspot/src/share/vm/gc/shared/gcTraceTime.inline.hpp
hotspot/src/share/vm/logging/log.cpp
hotspot/src/share/vm/logging/logTagSet.hpp
hotspot/src/share/vm/utilities/internalVMTests.cpp
--- a/hotspot/src/share/vm/gc/serial/defNewGeneration.cpp	Tue Mar 29 08:42:22 2016 +0200
+++ b/hotspot/src/share/vm/gc/serial/defNewGeneration.cpp	Tue Mar 29 09:36:43 2016 +0200
@@ -460,11 +460,11 @@
                   (HeapWord*)_virtual_space.high());
     gch->barrier_set()->resize_covered_region(cmr);
 
-    log_debug(gc, heap, ergo)(
+    log_debug(gc, ergo, heap)(
         "New generation size " SIZE_FORMAT "K->" SIZE_FORMAT "K [eden=" SIZE_FORMAT "K,survivor=" SIZE_FORMAT "K]",
         new_size_before/K, _virtual_space.committed_size()/K,
         eden()->capacity()/K, from()->capacity()/K);
-    log_trace(gc, heap, ergo)(
+    log_trace(gc, ergo, heap)(
         "  [allowed " SIZE_FORMAT "K extra for %d threads]",
           thread_increase_size/K, threads_count);
       }
--- a/hotspot/src/share/vm/gc/shared/gcTraceTime.inline.hpp	Tue Mar 29 08:42:22 2016 +0200
+++ b/hotspot/src/share/vm/gc/shared/gcTraceTime.inline.hpp	Tue Mar 29 09:36:43 2016 +0200
@@ -39,24 +39,27 @@
 
 template <LogLevelType Level, LogTagType T0, LogTagType T1, LogTagType T2, LogTagType T3, LogTagType T4, LogTagType GuardTag >
 void GCTraceTimeImpl<Level, T0, T1, T2, T3, T4, GuardTag>::log_start(jlong start_counter) {
-  if (Log<PREFIX_LOG_TAG(start), T0, T1, T2, T3>::is_level(Level)) {
+  STATIC_ASSERT(T0 != LogTag::__NO_TAG); // Need some tag to log on.
+  STATIC_ASSERT(T4 == LogTag::__NO_TAG); // Need to leave at least the last tag for the "start" tag in log_start()
+
+  // Get log with start tag appended (replace first occurrence of NO_TAG)
+  const LogTagType start_tag = PREFIX_LOG_TAG(start);
+  const LogTagType no_tag = PREFIX_LOG_TAG(_NO_TAG);
+  Log<T0,
+      T1 == no_tag ? start_tag : T1,
+      T1 != no_tag && T2 == no_tag ? start_tag : T2,
+      T2 != no_tag && T3 == no_tag ? start_tag : T3,
+      T3 != no_tag && T4 == no_tag ? start_tag : T4
+    > log;
+
+  if (log.is_level(Level)) {
     FormatBuffer<> start_msg("%s", _title);
     if (_gc_cause != GCCause::_no_gc) {
       start_msg.append(" (%s)", GCCause::to_string(_gc_cause));
     }
     start_msg.append(" (%.3fs)", TimeHelper::counter_to_seconds(start_counter));
     // Make sure to put the "start" tag last in the tag set
-    STATIC_ASSERT(T0 != LogTag::__NO_TAG); // Need some tag to log on.
-    STATIC_ASSERT(T4 == LogTag::__NO_TAG); // Need to leave at least the last tag for the "start" tag in log_start()
-    if (T1 == LogTag::__NO_TAG) {
-      Log<T0, PREFIX_LOG_TAG(start)>::template write<Level>("%s", start_msg.buffer());
-    } else if (T2 == LogTag::__NO_TAG) {
-      Log<T0, T1, PREFIX_LOG_TAG(start)>::template write<Level>("%s", start_msg.buffer());
-    } else if (T3 == LogTag::__NO_TAG) {
-      Log<T0, T1, T2, PREFIX_LOG_TAG(start)>::template write<Level>("%s", start_msg.buffer());
-    } else {
-      Log<T0, T1, T2, T3, PREFIX_LOG_TAG(start)>::template write<Level>("%s", start_msg.buffer());
-    }
+    log.template write<Level>("%s", start_msg.buffer());
   }
 }
 
--- a/hotspot/src/share/vm/logging/log.cpp	Tue Mar 29 08:42:22 2016 +0200
+++ b/hotspot/src/share/vm/logging/log.cpp	Tue Mar 29 09:36:43 2016 +0200
@@ -159,4 +159,49 @@
   assert(Test_logconfiguration_subscribe_triggered == 3, "subscription not triggered (3)");
 }
 
+void Test_logtagset_duplicates() {
+  for (LogTagSet* ts = LogTagSet::first(); ts != NULL; ts = ts->next()) {
+    char ts_name[512];
+    ts->label(ts_name, sizeof(ts_name), ",");
+
+    // verify that NO_TAG is never followed by a real tag
+    for (size_t i = 0; i < LogTag::MaxTags; i++) {
+      if (ts->tag(i) == LogTag::__NO_TAG) {
+        for (i++; i < LogTag::MaxTags; i++) {
+          assert(ts->tag(i) == LogTag::__NO_TAG,
+                 "NO_TAG was followed by a real tag (%s) in tagset %s",
+                 LogTag::name(ts->tag(i)), ts_name);
+        }
+      }
+    }
+
+    // verify that there are no duplicate tagsets (same tags in different order)
+    for (LogTagSet* other = ts->next(); other != NULL; other = other->next()) {
+      if (ts->ntags() != other->ntags()) {
+        continue;
+      }
+      bool equal = true;
+      for (size_t i = 0; i < ts->ntags(); i++) {
+        LogTagType tag = ts->tag(i);
+        if (!other->contains(tag)) {
+          equal = false;
+          break;
+        }
+      }
+      // Since tagsets are implemented using template arguments, using both of
+      // the (logically equivalent) tagsets (t1, t2) and (t2, t1) somewhere will
+      // instantiate two different LogTagSetMappings. This causes multiple
+      // tagset instances to be created for the same logical set. We want to
+      // avoid this to save time, memory and prevent any confusion around it.
+      if (equal) {
+        char other_name[512];
+        other->label(other_name, sizeof(other_name), ",");
+        assert(false, "duplicate LogTagSets found: '%s' vs '%s' "
+               "(tags must always be specified in the same order for each tagset)",
+               ts_name, other_name);
+      }
+    }
+  }
+}
+
 #endif // PRODUCT
--- a/hotspot/src/share/vm/logging/logTagSet.hpp	Tue Mar 29 08:42:22 2016 +0200
+++ b/hotspot/src/share/vm/logging/logTagSet.hpp	Tue Mar 29 09:36:43 2016 +0200
@@ -68,6 +68,10 @@
     return _ntags;
   }
 
+  LogTagType tag(size_t idx) const {
+    return _tag[idx];
+  }
+
   bool contains(LogTagType tag) const {
     for (size_t i = 0; _tag[i] != LogTag::__NO_TAG; i++) {
       if (tag == _tag[i]) {
--- a/hotspot/src/share/vm/utilities/internalVMTests.cpp	Tue Mar 29 08:42:22 2016 +0200
+++ b/hotspot/src/share/vm/utilities/internalVMTests.cpp	Tue Mar 29 09:36:43 2016 +0200
@@ -70,6 +70,7 @@
   run_unit_test(Test_log_length);
   run_unit_test(Test_configure_stdout);
   run_unit_test(Test_logconfiguration_subscribe);
+  run_unit_test(Test_logtagset_duplicates);
   run_unit_test(DirectivesParser_test);
   run_unit_test(Test_TempNewSymbol);
 #if INCLUDE_VM_STRUCTS