[foreign-abi] RFR: 8241017: Enhance AllocationScope to support "unbounded" mode

[Jorn Vernee]

On Fri, 13 Mar 2020 16:29:26 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch adds support for allocation scopes whose size is not known statically; as such these new unbounded
> allocation scope can be used in more dynamic use cases where e.g. the amount of memory to be allocated depends on the
> result of other native calls.  Internally, the bounded version works as before, and is the more optimized. The
> unbounded version is similar in spirit to the old Panama/foreign scope - where new segments are allocated depending on
> needs.  I've enhanced the existing test covering allocation scopes to also cover the unbounded variants, as well as the
> bounded.

Looks good, just some minor comments.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java line 39:

> 38:  * of the allocation scope is known statically. If an application knows before-hand how much memory it needs to
> allocate the values it needs, 39:  * using a <em>bound</em> allocation scope will typically provide better performances
> than independently allocating the memory 40:  * for each value (e.g. using {@link MemorySegment#allocateNative(long)}),
> or using an <em>unbounded</em> allocation scope.

Typo
Suggestion:

 * using a <em>bounded</em> allocation scope will typically provide better performances than independently allocating the
   memory

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java line 41:

> 40:  * for each value (e.g. using {@link MemorySegment#allocateNative(long)}), or using an <em>unbounded</em>
> allocation scope. 41:  * For this reason, using a bound allocation scope is recommended in cases where programs might
> need to emulate native stack allocation. 42:  */

Same typo
Suggestion:

 * For this reason, using a bounded allocation scope is recommended in cases where programs might need to emulate native
   stack allocation.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java line 46:

> 45:     /**
> 46:      * If this allocation scope is bound, returns the size, in bytes, of this allocation scope.
> 47:      * @return the size, in bytes, of this allocation scope (if available).

Suggestion:

     * If this allocation scope is bounded, returns the size, in bytes, of this allocation scope.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/UnboundedAllocationScope.java line 71:

> 70:                 sp = start + bytesSize;
> 71:                 size += Utils.alignUp(bytesSize, bytesAlignment);
> 72:                 return slice.baseAddress();

Aren't `sp` and `size` always the same? Are 2 separate fields really needed?

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

Marked as reviewed by jvernee (Committer).

PR: https://git.openjdk.java.net/panama-foreign/pull/53