Memory Segment efficient array handling

[Maurizio Cimadamore]

Thanks - I'll add this on my radar

Maurizio

On 15/06/2021 20:06, leerho wrote:
> Maurizio,
> I have updated the *MemoryCopy* and *TestMemoryCopy* to include 
> references to the new *MemorySegment::CopyFrom(...)* and with byte 
> swap capabilities.
>
> I have been able to test the code in my local environment using a mock 
> substitute for the new CopyFrom(), but alas, I could not get Jtreg to 
> work, so i am sending you the code as updated in 
> https://github.com/leerho/mcimadamore_panama-foreign 
> <https://urldefense.com/v3/__https://github.com/leerho/mcimadamore_panama-foreign__;!!GqivPVa7Brio!MZMNJXVSSBqQxv9G2nXnY-wpuBdgPpwIvBCl6CuzkZLWKaRjEAPnl1FHib5FeUyX3rF3rRU$>
>
> Please test it, when you have a chance.
>
> Lee.
>
>
> On Tue, Jun 15, 2021 at 5:43 AM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com 
> <mailto:maurizio.cimadamore at oracle.com>> wrote:
>
>
>     >> The reason why copying is slower for small segments is that, I
>     think, my
>     >> benchmark iterations run in 4-5ns and, at these levels, you are
>     >> sensitive to the cost of the liveness checks. There's not a lot
>     we can
>     >> do to eliminate that - it's the price of safety.
>     > OK that's true. But in my own Unsafe.copyMemory code wasn't
>     doing MemorySegment.address().toRawLongValue() not also doing the
>     liveness check?
>     True - so the liveness check probably that won't make a lot of
>     difference in your case. But note that the bound checks are still
>     applied in MemorySegment::copyMemory.
>     >
>     >> The good news is that in the case of heap segment wrappers, we know
>     >> exactly that the size is gonna fit inside an int (it's an array
>     after
>     >> all!), so we can remove the check. If we remove the flag check,
>     and just
>     >> set the segment to always be treated as "SMALL", we get the
>     following
>     >> results:
>     > Is this really true? What about a huge long[] array, that could
>     be (2^31-8 maximum index) * 8 bytesPerLong = 16 gigabytes? (or are
>     those limited by the Java standard in maximum array size?)
>     in the PR I've submitted I only tweaked byte segment wrappers. We
>     could
>     resort to more complex internal representation for other heap
>     segments,
>     but I believe the true way forward is to wait for the long
>     optimizations, and to completely remove the SMALL segment hacks.
>     >
>     > Sorry, I missed that class. I was opening the pull request and
>     only looked at the red/green changes in MemorySegment. I missed
>     the first class.
>     >
>     > The methods provided there exactly replace my three shapes in
>     the code. And on top they allow to swap. Side note: MemoryCopy in
>     the current PR has the swapping code commented out.
>     Yeah - Lee is working on that MemoryCopy class (and associated
>     test). I
>     have provided the "swappy" copyFrom overload in MemorySegment, I
>     believe
>     Lee will add support for that soon.
>     >
>     > Last question: why does
>     https://bugs.openjdk.java.net/browse/JDK-8223051
>     <https://bugs.openjdk.java.net/browse/JDK-8223051> not help for
>     the integer vs. long loop variables? Does this not allow to remove
>     the small vs. large segment distinction? I thought this change in
>     hotspot was added to support long loops, if it is in fact "small".
>
>     My understanding is that the change has been split in multiple
>     chunks.
>     The chunk we mostly depend on is this:
>
>     https://bugs.openjdk.java.net/browse/JDK-8259609
>     <https://bugs.openjdk.java.net/browse/JDK-8259609>
>
>     (in fact I'm coordinating with Roland, the author of that work,
>     and he's
>     testing the patch against a version of panama which doesn't have
>     workarounds for small segments).
>
>     I did run all Panama benchmarks after JDK-8223051 was pushed, and
>     removed various workarounds, but  there was still a performance gap.
>     Roland reassured me that the gap should disappear once JDK-8259609 is
>     pushed.
>
>     Maurizio
>
>     >
>     > Uwe
>     >
>