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

[Ty Young]

On 3/16/20 10:45 AM, Maurizio Cimadamore wrote:
> On Mon, 16 Mar 2020 03:37:52 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:
>
>>> Maurizio Cimadamore has updated the pull request incrementally with three additional commits since the last revision:
>>>
>>>   - Update src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java
>>>     
>>>     Co-Authored-By: Jorn Vernee <JornVernee at users.noreply.github.com>
>>>   - Update src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java
>>>     
>>>     Co-Authored-By: Jorn Vernee <JornVernee at users.noreply.github.com>
>>>   - Update src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/AllocationScope.java
>>>     
>>>     Co-Authored-By: Jorn Vernee <JornVernee at users.noreply.github.com>
>> Looks good!
>> I have some disagreements on the implementation. The expected behaviour,
>> in my mind anyway, is that the returned MemoryAddress is *ALWAYS* just
>> an offset of a larger, always same MemorySegment,
> The goal of AllocationScope is to provide some extra aid to those clients who need to allocate multiple segments which
> feature the same temporal bounds - e.g. so that they can be closed all at once.


If this is the intended purpose then it should, IMO, be changed to 
"BlockScope". "UnboundedAllocationScope" implies same-segment, dynamic 
allocation.


>   It gives you back an address to the
> allocated memory. How it's implemented inside, it's well, an implementation detail, and I don't really see why a client
> should assume certain behavior out of the addresses that it gets out of AllocationScope. Do you expect that all
> addresses given back to you via malloc are consecutive? I hope not!


Malloc? No. Realloc? Yes, at least for slicing.



>
> As for resizing a segment dynamically, I've said many times that the design centre for memory segment API is that to
> have immutable segments, so that bound checks can be better optimized. If that's the constraint, then, in order to have
> unbounded allocation, you have to allocate multiple segments and keep them in a list. Maybe we can do some tuning on
> the threshold and make the impl smarter at reusing the memory, but this is mostly beyond the point - as this is not
> supposed to be an highly efficient allocator - this is just supposed to be an help for clients building native
> application - one thing that we found was pretty common for clients to do when we started porting more and more code
> using the new bindings.


I'm in the same boat, as was said before, but this implementation isn't 
really desirable. It'll work for basic usage but if anyone goes looking 
for a value stored in the previous segment there will be issues...


I guess the advice then would be to use stdlib then, right?


Problem is that, at least on Arch Linux, it fails:


OpenJDK 64-Bit Server VM warning: You have loaded library 
/usr/lib/libc.so which might have disabled stack guard. The VM will try 
to fix the stack guard now.
It's highly recommended that you fix the library with 'execstack -c 
<libfile>', or link it with '-z noexecstack'.
Exception in thread "main" java.lang.UnsatisfiedLinkError: 
/usr/lib/libc.so: /usr/lib/libc.so: invalid ELF header
     at java.base/jdk.internal.loader.NativeLibraries.load(Native Method)
     at 
java.base/jdk.internal.loader.NativeLibraries$NativeLibraryImpl.open(NativeLibraries.java:289)
     at 
java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:176)
     at 
java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:118)
     at 
java.base/jdk.internal.loader.NativeLibraries.findFromPaths(NativeLibraries.java:224)
     at 
java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:216)
     at 
jdk.incubator.foreign/jdk.internal.foreign.LibrariesHelper.loadLibrary(LibrariesHelper.java:60)
     at 
jdk.incubator.foreign/jdk.incubator.foreign.LibraryLookup.ofLibrary(LibraryLookup.java:94)
     at org.goliath.crosspoint.stdlib.GNUC.<init>(GNUC.java:37)
     at org.goliath.crosspoint.stdlib.GNUC.getInstance(GNUC.java:21)
     at org.goliath.bindings.nvml.main.Test.main(Test.java:70)


The only information I could find is that maybe libc is 32-bit or 
something... but it's located in /usr/lib which is 64-bit...



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