Directions on fixing JDK-8297285

[Ashutosh Mehra]

>
>

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() ?
>



I was trying to avoid adding another check in the allocation path
especially since it would only be used during init.
But if there is no other easy way to fix it, I can live with it.

Thanks,
Ashutosh Mehra


On Thu, Nov 24, 2022 at 10:49 AM Kennke, Roman <rkennke at amazon.de> wrote:

> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/shenandoah-dev/attachments/20221124/0bc88d27/attachment.htm>