Memory Segment efficient array handling
leerho
leerho at gmail.com
Thu Jun 17 18:51:40 UTC 2021
Initializing the float and double segments with integers {0,1,...} is OK.
My only thought was to have at least one test where all the bytes had
values other than zero -- to make sure all the bytes were being swapped
correctly. Swapping bytes of zero is hard to determine correctness :-)
What you did allows leverage of the same code for all the primitives, which
simplifies things a lot.
Lee.
On Thu, Jun 17, 2021 at 10:59 AM Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:
>
> On 17/06/2021 18:56, leerho wrote:
>
> 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.
>
> Hopefully the test logic should be the same. The only think in the test
> which did not survive was the fact that for float and doubles, the old
> logic started the segment with Math.PI. The new logic just works in bytes
> and always starts at 0.
>
> We could tweak that if required, but IMHO initializing segment with bytes
> should always be good enough?
>
>
> I am very grateful for the time you spent on this!
>
> You're welcome - thanks a lot for the help on this one!
>
> Maurizio
>
>
> 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!JGhJxrE53VOn-cTxQVrSqgTu2RNga-LHbmFznJnCRboLnzElMBFILU_QjDRyz-R-HaFEm0A$>
>> >> <
>> 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
>> >> >
>> >>
>>
>
More information about the panama-dev
mailing list