Shenandoah: Question about asserts

Per Liden per.liden at oracle.com
Thu Sep 9 09:48:57 UTC 2021


Hi Shenandoah-folks,

When doing some random testing of 
https://github.com/openjdk/jdk/pull/5410 I noticed that Shenandoah ran 
into a few asserts during heap dumping. The asserts check that we're 
executing in the VM thread. However, I wonder if these asserts might be 
a bit too aggressive, or if there's a reason these functions must only 
be executed by the VM thread? I couldn't see any obvious reason, so I 
simply removed the offending asserts (see patch below) and nothing 
obvious seems to break (heap dumping tests now pass, etc).

So, would you mind if I remove these asserts, or are they really needed 
for some reason I've missed?

cheers,
Per


diff --git a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp 
b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
index ca9afd7ad4f..623ca9574a2 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahCodeRoots.cpp
@@ -355,7 +355,6 @@ 
ShenandoahCodeRootsIterator::ShenandoahCodeRootsIterator() :
          _par_iterator(CodeCache::heaps()),
          _table_snapshot(NULL) {
    assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint");
-  assert(!Thread::current()->is_Worker_thread(), "Should not be 
acquired by workers");
    CodeCache_lock->lock_without_safepoint_check();
    _table_snapshot = 
ShenandoahCodeRoots::table()->snapshot_for_iteration();
  }
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp 
b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
index c0469a71d7b..261920a9d7a 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp
@@ -258,7 +258,6 @@ 
ShenandoahHeapIterationRootScanner::ShenandoahHeapIterationRootScanner() :
   }

   void ShenandoahHeapIterationRootScanner::roots_do(OopClosure* oops) {
-   assert(Thread::current()->is_VM_thread(), "Only by VM thread");
     // Must use _claim_none to avoid interfering with concurrent CLDG 
iteration
     CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
     MarkingCodeBlobClosure code(oops, 
!CodeBlobToOopClosure::FixRelocations);
diff --git 
a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp 
b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
index 3f614d1e208..b2897d380aa 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.inline.hpp
@@ -86,9 +86,8 @@ ShenandoahClassLoaderDataRoots<CONCURRENT, 
SINGLE_THREADED>::ShenandoahClassLoad
      ClassLoaderDataGraph_lock->lock();
    }

-  // Non-concurrent mode only runs at safepoints by VM thread
+  // Non-concurrent mode only runs at safepoints
    assert(CONCURRENT || SafepointSynchronize::is_at_safepoint(), "Must 
be at a safepoint");
-  assert(CONCURRENT || Thread::current()->is_VM_thread(), "Can only be 
done by VM thread");
  }

  template <bool CONCURRENT, bool SINGLE_THREADED>



More information about the hotspot-gc-dev mailing list