8196876: OopStorage::assert_at_safepoint clashes with assert_at_safepoint macros in g1CollectedHeap.hpp
authorkbarrett
Sat, 03 Mar 2018 23:56:08 -0500
changeset 49333 489f1dd40582
parent 49332 cd21aff24069
child 49334 aefee96e2b90
8196876: OopStorage::assert_at_safepoint clashes with assert_at_safepoint macros in g1CollectedHeap.hpp Summary: Add shared safepoint state assertion macros. Reviewed-by: coleenp, eosterlund
src/hotspot/share/gc/g1/dirtyCardQueue.cpp
src/hotspot/share/gc/g1/g1Allocator.cpp
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp
src/hotspot/share/gc/g1/g1CollectionSet.cpp
src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp
src/hotspot/share/gc/shared/oopStorage.cpp
src/hotspot/share/gc/shared/oopStorage.hpp
src/hotspot/share/gc/shared/oopStorage.inline.hpp
src/hotspot/share/runtime/safepoint.hpp
--- a/src/hotspot/share/gc/g1/dirtyCardQueue.cpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/dirtyCardQueue.cpp	Sat Mar 03 23:56:08 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, 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
@@ -250,7 +250,7 @@
 }
 
 bool DirtyCardQueueSet::apply_closure_during_gc(CardTableEntryClosure* cl, uint worker_i) {
-  assert_at_safepoint(false);
+  assert_at_safepoint();
   return apply_closure_to_completed_buffer(cl, worker_i, 0, true);
 }
 
--- a/src/hotspot/share/gc/g1/g1Allocator.cpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1Allocator.cpp	Sat Mar 03 23:56:08 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2018, 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
@@ -87,7 +87,7 @@
 }
 
 void G1DefaultAllocator::init_gc_alloc_regions(EvacuationInfo& evacuation_info) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   _survivor_is_full = false;
   _old_is_full = false;
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Sat Mar 03 23:56:08 2018 -0500
@@ -566,7 +566,7 @@
 }
 
 void G1CollectedHeap::begin_archive_alloc_range(bool open) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
   if (_archive_allocator == NULL) {
     _archive_allocator = G1ArchiveAllocator::create_allocator(this, open);
   }
@@ -580,7 +580,7 @@
 }
 
 HeapWord* G1CollectedHeap::archive_mem_allocate(size_t word_size) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
   assert(_archive_allocator != NULL, "_archive_allocator not initialized");
   if (is_archive_alloc_too_large(word_size)) {
     return NULL;
@@ -590,7 +590,7 @@
 
 void G1CollectedHeap::end_archive_alloc_range(GrowableArray<MemRegion>* ranges,
                                               size_t end_alignment_in_bytes) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
   assert(_archive_allocator != NULL, "_archive_allocator not initialized");
 
   // Call complete_archive to do the real work, filling in the MemRegion
@@ -983,7 +983,7 @@
 
 HeapWord* G1CollectedHeap::attempt_allocation_at_safepoint(size_t word_size,
                                                            bool expect_null_mutator_alloc_region) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
   assert(!_allocator->has_mutator_alloc_region() || !expect_null_mutator_alloc_region,
          "the current alloc region was unexpectedly found to be non-NULL");
 
@@ -1154,7 +1154,7 @@
 
 bool G1CollectedHeap::do_full_collection(bool explicit_gc,
                                          bool clear_all_soft_refs) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   if (GCLocker::check_active_before_gc()) {
     // Full GC was not completed.
@@ -1295,7 +1295,7 @@
 
 HeapWord* G1CollectedHeap::satisfy_failed_allocation(size_t word_size,
                                                      bool* succeeded) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   // Attempts to allocate followed by Full GC.
   HeapWord* result =
@@ -1347,7 +1347,7 @@
 // allocated block, or else "NULL".
 
 HeapWord* G1CollectedHeap::expand_and_allocate(size_t word_size) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   _verifier->verify_region_sets_optional();
 
@@ -2817,7 +2817,7 @@
 
 bool
 G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
   guarantee(!is_gc_active(), "collection is not reentrant");
 
   if (GCLocker::check_active_before_gc()) {
@@ -4847,7 +4847,7 @@
 };
 
 void G1CollectedHeap::eagerly_reclaim_humongous_regions() {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   if (!G1EagerReclaimHumongousObjects ||
       (!_has_humongous_reclaim_candidates && !log_is_enabled(Debug, gc, humongous))) {
@@ -5003,7 +5003,7 @@
 };
 
 void G1CollectedHeap::tear_down_region_sets(bool free_list_only) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   if (!free_list_only) {
     TearDownRegionSetsClosure cl(&_old_set);
@@ -5077,7 +5077,7 @@
 };
 
 void G1CollectedHeap::rebuild_region_sets(bool free_list_only) {
-  assert_at_safepoint(true /* should_be_vm_thread */);
+  assert_at_safepoint_on_vm_thread();
 
   if (!free_list_only) {
     _eden.clear();
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -364,17 +364,11 @@
                                    "should not be at a safepoint"));          \
   } while (0)
 
-#define assert_at_safepoint(_should_be_vm_thread_)                            \
+#define assert_at_safepoint_on_vm_thread()                                    \
   do {                                                                        \
-    assert(SafepointSynchronize::is_at_safepoint() &&                         \
-              ((_should_be_vm_thread_) == Thread::current()->is_VM_thread()), \
-           heap_locking_asserts_params("should be at a safepoint"));          \
-  } while (0)
-
-#define assert_not_at_safepoint()                                             \
-  do {                                                                        \
-    assert(!SafepointSynchronize::is_at_safepoint(),                          \
-           heap_locking_asserts_params("should not be at a safepoint"));      \
+    assert_at_safepoint();                                                    \
+    assert(Thread::current_or_null() != NULL, "no current thread");           \
+    assert(Thread::current()->is_VM_thread(), "current thread is not VM thread"); \
   } while (0)
 
 protected:
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -85,12 +85,12 @@
 }
 
 inline void G1CollectedHeap::reset_gc_time_stamp() {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
   _gc_time_stamp = 0;
 }
 
 inline void G1CollectedHeap::increment_gc_time_stamp() {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
   ++_gc_time_stamp;
 }
 
--- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Sat Mar 03 23:56:08 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, 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
@@ -80,7 +80,7 @@
 
 void G1CollectionSet::init_region_lengths(uint eden_cset_region_length,
                                           uint survivor_cset_region_length) {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   _eden_region_length     = eden_cset_region_length;
   _survivor_region_length = survivor_cset_region_length;
@@ -103,7 +103,7 @@
 
 // Add the heap region at the head of the non-incremental collection set
 void G1CollectionSet::add_old_region(HeapRegion* hr) {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   assert(_inc_build_state == Active, "Precondition");
   assert(hr->is_old(), "the region should be old");
@@ -167,7 +167,7 @@
 }
 
 void G1CollectionSet::clear() {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
   _collection_set_cur_length = 0;
 }
 
@@ -314,7 +314,7 @@
 };
 
 bool G1CollectionSet::verify_young_ages() {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   G1VerifyYoungAgesClosure cl;
   iterate(&cl);
@@ -541,7 +541,7 @@
 };
 
 void G1CollectionSet::verify_young_cset_indices() const {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   G1VerifyYoungCSetIndicesClosure cl(_collection_set_cur_length);
   iterate(&cl);
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, 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
@@ -58,7 +58,7 @@
 #ifndef PRODUCT
 template<typename Fn>
 inline void G1CMMarkStack::iterate(Fn fn) const {
-  assert_at_safepoint(true);
+  assert_at_safepoint_on_vm_thread();
 
   size_t num_chunks = 0;
 
--- a/src/hotspot/share/gc/shared/oopStorage.cpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/shared/oopStorage.cpp	Sat Mar 03 23:56:08 2018 -0500
@@ -283,12 +283,6 @@
   return NULL;
 }
 
-#ifdef ASSERT
-void OopStorage::assert_at_safepoint() {
-  assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
-}
-#endif // ASSERT
-
 //////////////////////////////////////////////////////////////////////////////
 // Allocation
 //
@@ -728,7 +722,9 @@
 }
 
 void OopStorage::BasicParState::ensure_iteration_started() {
-  if (!_concurrent) assert_at_safepoint();
+  if (!_concurrent) {
+    assert_at_safepoint();
+  }
   assert(!_concurrent || _storage->_concurrent_iteration_active, "invariant");
   // Ensure _next_block is not the not_started_marker, setting it to
   // the _active_head to start the iteration if necessary.
--- a/src/hotspot/share/gc/shared/oopStorage.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/shared/oopStorage.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -241,8 +241,6 @@
   void delete_empty_block(const Block& block);
   bool reduce_deferred_updates();
 
-  static void assert_at_safepoint() NOT_DEBUG_RETURN;
-
   template<typename F, typename Storage>
   static bool iterate_impl(F f, Storage* storage);
 
--- a/src/hotspot/share/gc/shared/oopStorage.inline.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/gc/shared/oopStorage.inline.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -30,6 +30,7 @@
 #include "metaprogramming/conditional.hpp"
 #include "metaprogramming/isConst.hpp"
 #include "oops/oop.hpp"
+#include "runtime/safepoint.hpp"
 #include "utilities/count_trailing_zeros.hpp"
 #include "utilities/debug.hpp"
 #include "utilities/globalDefinitions.hpp"
--- a/src/hotspot/share/runtime/safepoint.hpp	Fri Mar 02 17:33:59 2018 -0800
+++ b/src/hotspot/share/runtime/safepoint.hpp	Sat Mar 03 23:56:08 2018 -0500
@@ -209,6 +209,20 @@
   }
 };
 
+// Some helper assert macros for safepoint checks.
+
+#define assert_at_safepoint()                                           \
+  assert(SafepointSynchronize::is_at_safepoint(), "should be at a safepoint")
+
+#define assert_at_safepoint_msg(...)                                    \
+  assert(SafepointSynchronize::is_at_safepoint(), __VA_ARGS__)
+
+#define assert_not_at_safepoint()                                       \
+  assert(!SafepointSynchronize::is_at_safepoint(), "should not be at a safepoint")
+
+#define assert_not_at_safepoint_msg(...)                                \
+  assert(!SafepointSynchronize::is_at_safepoint(), __VA_ARGS__)
+
 // State class for a thread suspended at a safepoint
 class ThreadSafepointState: public CHeapObj<mtInternal> {
  public: