Directions on fixing JDK-8297285

[Kennke, Roman]

Hi Ashu,

In ShenandoahPacer::pace_for_alloc(), there is a check like this:

   // Threads that are attaching should not block at all: they are not
   // fully initialized yet. Blocking them would be awkward.
   // This is probably the path that allocates the thread oop itself.
   if (JavaThread::current()->is_attaching_via_jni()) {
     return;
   }

Maybe the right thing to do is to add a check for whether or not the 
thread is in threads list or is_active_Java_thread() ?

Also, CCing Aleksey, because he originally wrote this code, IIRC.

Thanks,
Roman


> Any suggestions on the problem mentioned in the previous email?
> I would like to add that the problem is likely to become more apparent 
> with https://bugs.openjdk.org/browse/JDK-8293650 
> <https://bugs.openjdk.org/browse/JDK-8293650> as it would increase heap 
> utilization during VM init.
> 
> Thanks,
> Ashutosh Mehra
> 
> 
> ---------- Forwarded message ---------
> From: *Ashutosh Mehra* <asmehra at redhat.com <mailto:asmehra at redhat.com>>
> Date: Mon, Nov 21, 2022 at 4:09 PM
> Subject: Directions on fixing JDK-8297285
> To: <shenandoah-dev at openjdk.org <mailto:shenandoah-dev at openjdk.org>>
> 
> 
> Hi,
> 
> I am trying to find a way to fix JDK-8297285: Shenandoah pacing causes 
> assertion failure during VM initialization.
> 
> The scenario is during VM init if the main thread exhausts the 
> allocation budget, then the shenandoah pacer would cause it to wait. The 
> call to wait can hit assertion self->is_active_Java_thread() in 
> Monitor::wait() as the main thread is not yet added to the thread list, 
> and therefore is_active_Java_thread() returns false.
> 
> The approach I have in mind is to delay initialization of 
> ShenandoahPacer until the main thread is considered an active Java 
> thread. But there is no hook to capture that event in the GC. The 
> closest I found is `CollectedHeap::post_initialize()`. The comments for 
> `CollectedHeap::post_initialize()` indicate this /may/ be the right 
> place to initialize the ShenandoahPacer:
> 
> ```
>    // In many heaps, there will be a need to perform some initialization 
> activities
>    // after the Universe is fully formed, but before general heap 
> allocation is allowed.
>    // This is the correct place to place such initialization methods.
>    virtual void post_initialize();
> ```
> 
> However, there are still some VM init activities between the call to 
> CollectedHeap::post_initialize() in universe_post_init and the call to 
> Threads::add(main_thread) in Threads::create_vm(). So initializing in 
> CollectedHeap::post_initialize() can still break if enough heap 
> allocations happen during those activities.
> I am wondering if there is a better place to initialize the 
> ShenandoahPacer, or another approach to fix this situation.
> Here is the current set of changes [0] that initialize ShenandoahPacer 
> in  CollectedHeap::post_initialize().
> 
> [0] 
> https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285 <https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285>
> 
> Thanks,
> Ashutosh Mehra



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879