<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Maybe the right thing to do is to add a check for whether or not the<br>thread is in threads list or is_active_Java_thread() ?<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><div><br></div><div>I was trying to avoid adding another check in the allocation path especially since it would only be used during init.</div><div>But if there is no other easy way to fix it, I can live with it.<br></div><div><br></div><div>Thanks,</div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Ashutosh Mehra</div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 24, 2022 at 10:49 AM Kennke, Roman <<a href="mailto:rkennke@amazon.de">rkennke@amazon.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Ashu,<br>
<br>
In ShenandoahPacer::pace_for_alloc(), there is a check like this:<br>
<br>
  Â // Threads that are attaching should not block at all: they are not<br>
  Â // fully initialized yet. Blocking them would be awkward.<br>
  Â // This is probably the path that allocates the thread oop itself.<br>
  Â if (JavaThread::current()->is_attaching_via_jni()) {<br>
  Â  Â return;<br>
  Â }<br>
<br>
Maybe the right thing to do is to add a check for whether or not the <br>
thread is in threads list or is_active_Java_thread() ?<br>
<br>
Also, CCing Aleksey, because he originally wrote this code, IIRC.<br>
<br>
Thanks,<br>
Roman<br>
<br>
<br>
> Any suggestions on the problem mentioned in the previous email?<br>
> I would like to add that the problem is likely to become more apparent <br>
> with <a href="https://bugs.openjdk.org/browse/JDK-8293650" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8293650</a> <br>
> <<a href="https://bugs.openjdk.org/browse/JDK-8293650" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8293650</a>> as it would increase heap <br>
> utilization during VM init.<br>
> <br>
> Thanks,<br>
> Ashutosh Mehra<br>
> <br>
> <br>
> ---------- Forwarded message ---------<br>
> From: *Ashutosh Mehra* <<a href="mailto:asmehra@redhat.com" target="_blank">asmehra@redhat.com</a> <mailto:<a href="mailto:asmehra@redhat.com" target="_blank">asmehra@redhat.com</a>>><br>
> Date: Mon, Nov 21, 2022 at 4:09 PM<br>
> Subject: Directions on fixing JDK-8297285<br>
> To: <<a href="mailto:shenandoah-dev@openjdk.org" target="_blank">shenandoah-dev@openjdk.org</a> <mailto:<a href="mailto:shenandoah-dev@openjdk.org" target="_blank">shenandoah-dev@openjdk.org</a>>><br>
> <br>
> <br>
> Hi,<br>
> <br>
> I am trying to find a way to fix JDK-8297285: Shenandoah pacing causes <br>
> assertion failure during VM initialization.<br>
> <br>
> The scenario is during VM init if the main thread exhausts the <br>
> allocation budget, then the shenandoah pacer would cause it to wait. The <br>
> call to wait can hit assertion self->is_active_Java_thread() in <br>
> Monitor::wait() as the main thread is not yet added to the thread list, <br>
> and therefore is_active_Java_thread() returns false.<br>
> <br>
> The approach I have in mind is to delay initialization of <br>
> ShenandoahPacer until the main thread is considered an active Java <br>
> thread. But there is no hook to capture that event in the GC. The <br>
> closest I found is `CollectedHeap::post_initialize()`. The comments for <br>
> `CollectedHeap::post_initialize()` indicate this /may/ be the right <br>
> place to initialize the ShenandoahPacer:<br>
> <br>
> ```<br>
>  Â  // In many heaps, there will be a need to perform some initialization <br>
> activities<br>
>  Â  // after the Universe is fully formed, but before general heap <br>
> allocation is allowed.<br>
>  Â  // This is the correct place to place such initialization methods.<br>
>  Â  virtual void post_initialize();<br>
> ```<br>
> <br>
> However, there are still some VM init activities between the call to <br>
> CollectedHeap::post_initialize() in universe_post_init and the call to <br>
> Threads::add(main_thread) in Threads::create_vm(). So initializing in <br>
> CollectedHeap::post_initialize() can still break if enough heap <br>
> allocations happen during those activities.<br>
> I am wondering if there is a better place to initialize the <br>
> ShenandoahPacer, or another approach to fix this situation.<br>
> Here is the current set of changes [0] that initialize ShenandoahPacer <br>
> in  CollectedHeap::post_initialize().<br>
> <br>
> [0] <br>
> <a href="https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285</a> <<a href="https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/compare/master...ashu-mehra:jdk:JDK-8297285</a>><br>
> <br>
> Thanks,<br>
> Ashutosh Mehra<br>
<br>
<br>
<br>
Amazon Development Center Germany GmbH<br>
Krausenstr. 38<br>
10117 Berlin<br>
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss<br>
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B<br>
Sitz: Berlin<br>
Ust-ID: DE 289 237 879<br>
<br>
<br>
</blockquote></div>