diff src/share/vm/gc_implementation/g1/satbQueue.cpp @ 1317:d4197f8d516a

6935821: G1: threads created during marking do not active their SATB queues Summary: Newly-created threads always had the active field of their SATB queue initialized to false, even if they were created during marking. As a result, updates from threads created during a marking cycle were never enqueued and never processed. The fix includes remaining a method from active() to is_active() for readability and naming consistency. Reviewed-by: ysr, johnc
author tonyp
date Thu, 18 Mar 2010 12:14:59 -0400
parents 44f61c24ddab
children c18cbe5936b8
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/satbQueue.cpp	Mon Mar 22 02:40:53 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/satbQueue.cpp	Thu Mar 18 12:14:59 2010 -0400
@@ -82,9 +82,57 @@
   t->satb_mark_queue().handle_zero_index();
 }
 
-void SATBMarkQueueSet::set_active_all_threads(bool b) {
+#ifdef ASSERT
+void SATBMarkQueueSet::dump_active_values(JavaThread* first,
+                                          bool expected_active) {
+  gclog_or_tty->print_cr("SATB queue active values for Java Threads");
+  gclog_or_tty->print_cr(" SATB queue set: active is %s",
+                         (is_active()) ? "TRUE" : "FALSE");
+  gclog_or_tty->print_cr(" expected_active is %s",
+                         (expected_active) ? "TRUE" : "FALSE");
+  for (JavaThread* t = first; t; t = t->next()) {
+    bool active = t->satb_mark_queue().is_active();
+    gclog_or_tty->print_cr("  thread %s, active is %s",
+                           t->name(), (active) ? "TRUE" : "FALSE");
+  }
+}
+#endif // ASSERT
+
+void SATBMarkQueueSet::set_active_all_threads(bool b,
+                                              bool expected_active) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint.");
+  JavaThread* first = Threads::first();
+
+#ifdef ASSERT
+  if (_all_active != expected_active) {
+    dump_active_values(first, expected_active);
+
+    // I leave this here as a guarantee, instead of an assert, so
+    // that it will still be compiled in if we choose to uncomment
+    // the #ifdef ASSERT in a product build. The whole block is
+    // within an #ifdef ASSERT so the guarantee will not be compiled
+    // in a product build anyway.
+    guarantee(false,
+              "SATB queue set has an unexpected active value");
+  }
+#endif // ASSERT
   _all_active = b;
-  for(JavaThread* t = Threads::first(); t; t = t->next()) {
+
+  for (JavaThread* t = first; t; t = t->next()) {
+#ifdef ASSERT
+    bool active = t->satb_mark_queue().is_active();
+    if (active != expected_active) {
+      dump_active_values(first, expected_active);
+
+      // I leave this here as a guarantee, instead of an assert, so
+      // that it will still be compiled in if we choose to uncomment
+      // the #ifdef ASSERT in a product build. The whole block is
+      // within an #ifdef ASSERT so the guarantee will not be compiled
+      // in a product build anyway.
+      guarantee(false,
+                "thread has an unexpected active value in its SATB queue");
+    }
+#endif // ASSERT
     t->satb_mark_queue().set_active(b);
   }
 }