[foreign-memaccess+abi] RFR: 8268743: Require a better way for copying data between MemorySegments and on-heap arrays

[Uwe Schindler]

On Tue, 15 Jun 2021 11:56:26 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> After some investigation, it seems that escape analysis is defeated in cases where a new heap segment is created fresh just before performing a bulk copy.
> 
> This is caused by the fact that, on segment creation, we perform this test:
> 
> 
> static int defaultAccessModes(long size) {
>         return (enableSmallSegments && size < Integer.MAX_VALUE) ?
>                 SMALL : 0;
>     }
> 
> 
> To make sure that segments whose size fits in an `int` do not incur in penalties associated with lack of optimizations over long loop bound check optimizations.
> 
> Unfortunately, this logic is control flow logic, and control flow disables escape analysis optimizations.
> 
> For segment wrappers around byte arrays we can workaround by removing the check (all byte segments are small by definition, since there's a 1-1 mapping between logical elements and physical bytes). For other segment kinds we cannot do much.
> 
> While it would be possible, in principle, to resort to more complex bound checks for heap segments, we believe the way forward is to eliminate the need for "small" segments, which will be possible once the PR below is completed:
> 
> https://github.com/openjdk/jdk/pull/2045

Hi Maurizio,
it's not possible today, because I have a talk at a conference later, but tomorrrow, I can for sure test the repo provided by you.

To call the new static MemorySegment.copy() method, I still need to wrap the byte[] array, so I should merge in THIS pull request? From looking at your repository it does not seem to have the HeapMemorySegment SMALL change.

Maybe you can also add this small change from this PR also to your benchmark repo.

About the liveness check: In my unsafe copy implementation, I called `segment.address().toRawLongAddress()`, so it was also doing the liveness checks. This had no effect on the performance. The only difference is the additional fencing after the whole copyMemory operation in ScopedMemoryAccess.

> The issue with memory copy is that memory copy _is_ typically used in a
> standalone fashion, e.g. outside of a loop. I'm pretty sure that if I
> repeat the benchmark by wrapping it in a loop and do like 100
> iterations, the difference with unsafe will disappear. But that's _not_
> how users will use `copyFrom`.

In most cases this is true. Actually we have sometimes a loop, but very seldom: We only use mmap when reading index files, but not for writing new index files, which are just streamed to a classical OutputStream. Sometimes we have to copy data to an OutputStream that was previously read from a MMapped InputStream (happens when merging Lucene index segments). In that case the code will call IndexInput.readBytes(...) and pass the byte array to OutputStream in the usual and well-known copy-loop. But that's of course not the usual case (especially not in our benchmark, because it does not build new indexes); it just executes queries.

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

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