RFR: 8342444: Shenandoah: Uncommit regions from a separate, STS aware thread

[Aleksey Shipilev]

On Mon, 11 Nov 2024 21:18:41 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> After reading more of this PR, I believe we need the heap lock to get a reliable signal of bitmap_slice_committed().  But I believe we do not need the heap lock for ctx->clear_bitmap(region) so would prefer to move that outside the lock, unless I am misunderstanding.
>
> Hmm, I'm not sure we can do that. Prior to this change, the control thread performed both clearing the bitmap and uncommitting the region's bitmap, so they could never happen concurrently. With this change, a separate thread could perform the uncommit. Consider:
> 
> 1. Control thread takes heap lock, observes that bitmap slice for region A is committed
> 2. Control thread releases heap lock, begins clearing bitmap (writing zeros to bitmap slice)
> 3. Uncommit thread takes heap lock, believes it must uncommit region A
> 4. Uncommit thread uncommits bitmap slice for region A
> 5. Segfault in Control Thread
> 
> I do _believe_ if we had a per region lock, it would be useful here. Holding a lock over the entire heap for this feels like overkill. Or, we could schedule the uncommit so that it does not occur during a GC cycle.

So, wait a sec. This code is in `ShenandoahResetBitmapTask`, so it can run in parallel. Putting a lock here inhibits parallelism. I understand the failure mode, but I think we should really be optimizing for the case when `ShenandoahUncommit` is not enabled (e.g. `-Xmx` == `-Xms`).

Sounds like there is a hassle in allowing concurrent uncommit to overlap with the GC cycle. In addition to this particular problem, we might be stealing cycles from the GC threads and take additional TTSP lag to park the uncommitter for the in-cycle GC pauses. I have no clear solution for this yet, but I think we need to explore if we can suspend the uncommit before going into GC cycle...

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22019#discussion_r1838568215