RFC: static bulk copy methods
John Rose
john.r.rose at oracle.com
Thu Jun 10 18:10:16 UTC 2021
On Jun 10, 2021, at 7:28 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com<mailto:maurizio.cimadamore at oracle.com>> wrote:
```
MemorySegment.copyFrom(MemorySegment src, int elemSize, boolean swap) // 1
MemorySegment.copyFrom(MemorySegment src, ValueLayout elemLayout, ByteOrder order) // 2
MemorySegment.copyFrom(MemorySegment src, ValueLayout srcElementLayout, ValueLayout dstElementLayout) // 3
```
The first is merely a wrapper around unsafe. It's a good fallback, but seems very low level, even for our API; specifically, it seems we could do something to avoid that "swap" parameter.
Just to state the obvious: “swap” parameters all by
themselves are dangerous because they pertain to
the operation but are not derived directly from
the source alone or the destination alone, but
rather from an XOR of properties of the source
and destination. But when reading code a
maintainer will usually mentally assign the
swap to either the source or destination.
This leads to maintenance problems in larger
systems where chains of copies, each with its
own swap parameter, are “fixed” by flipping a
coin (in effect) and complementing one of the
swap parameters, hoping for success. It sounds
silly, but that’s how byte-order bugs get fixed,
if the copy operations are gated by swap booleans,
rather than by properties of the data chunks
being copies.
We chased swap bugs in HotSpot like this,
across various modules written by people
with different mental models of byte order,
literally for years back in the day. I suspect
there is still a latent bug or two, masked by
“tweaks” that put the right (odd or even)
number of swap operations in each chain
that actually executes.
The road to sanity is through the option
where both source and destination get to
declare their “polarity” and the actual
operation just XORs those polarities on
the fly.
That’s my take, based on painful memories!
— John
P.S. While I prefer #3, I find it odd that the
two “polarity” descriptors are bundled
together at the end of the argument list,
and *in the reverse order* of the main
arguments they describe. That seems to
be a different kind of bug-attractor,
since it’s hard to see in the code if the
order has been reversed.
I like to break up such “dancing pairs”
of similar arguments if I can. Maybe:
MemorySegment.copyWithLayoutsFrom(ValueLayout dstElementLayout, MemorySegment src, ValueLayout srcElementLayout)
More information about the panama-dev
mailing list