Memory Segment efficient array handling

[leerho]

Maurizio,
Wow! Thanks!

You clearly put a lot of work into these changes!  You completely rewrote
the test class using tools that I was not aware of as well as
simplifications of the ultimate code.  I was pleased that at least the
basic concept of the tests survived.

I am very grateful for the time you spent on this!

Cheers,
Lee.



On Thu, Jun 17, 2021 at 8:44 AM Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:

> I've finalized the PR, it is now a full PR which is ready for review. I
> did some passes over the code and fix some things here and there,
> hopefully nothing too controversial.
>
> Cheers
> Maurizio
>
> On 16/06/2021 16:56, Maurizio Cimadamore wrote:
> > Hi Lee,
> > I've updated the PR with your latest changes. Everything works fine -
> > there were some trailing whitespaces that I fixed, other than that the
> > test works ok.
> >
> > 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
> >>     >
> >>
>