On Mon, 27 Jan 2025 22:06:58 GMT, Jorn Vernee <[email protected]> wrote:
> Currently, to free the memory allocated in a confined arena, we keep track of
> a list of 'cleanup actions', stored in linked list format in a so-called
> `ResourceList`, attached to the scope of the arena. When the scope is closed,
> we loop over all the entries in this resource list, and run all the cleanup
> actions one by one.
>
> However, due to this linked list format, plus the control flow introduced by
> the cleanup loop, C2's escape analysis can not keep track of the nodes of
> this linked list (`ResourceList.ResourceCleanup`), and as a result, they can
> not be scalar replaced.
>
> We can prevent just the first `ResourceCleanup` instance from escaping, by
> pulling out the first element of the list into a separate field. I also tried
> a setup where I had 2 separate fields for the first 2 elements, as well as a
> setup with an array with a fixed set of elements. While these also worked to
> prevent the first node from escaping, they were not able to provide the same
> benefit for multiple resource cleanup instances. Nevertheless, avoiding the
> allocation of the first element is relatively simple, and seems like a
> low-hanging fruit.
>
> I've changed the `AllocTest` benchmark a bit so that we don't return the
> `MemorySegment` in `alloc_confined`, which would make it always escape. That
> way, we can use this existing benchmark to test whether there are any
> allocations when calling `allocate` on a confined arena. This matches what we
> were doing in the other benchmark methods in the same class.
This somewhat reminds me of #22043, don't know how applicable the block trick
is for cleanup, but it doesn't help escape analysis...
src/java.base/share/classes/jdk/internal/foreign/ConfinedSession.java line 112:
> 110: if (fst != ResourceCleanup.CLOSED_LIST) {
> 111: ResourceCleanup prev = fst;
> 112: fst = ResourceCleanup.CLOSED_LIST;
Is there a reason why usage of fst here doesn't prevent successful escape
analysis?
-------------
PR Review: https://git.openjdk.org/jdk/pull/23321#pullrequestreview-2576802578
PR Review Comment: https://git.openjdk.org/jdk/pull/23321#discussion_r1931348181
