[foreign-memaccess] [Rev 01] RFR: Add MemorySegment::fill

[Chris Hegarty]

On Thu, 14 May 2020 13:25:18 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Marked as reviewed by psandoz (Committer).
>
> Thought a bit more about this.
> 
> There's an additional angle to the problem of segment vs. address which we didn't discuss. While all segments have
> a *base address*, the reverse is not true: if an address is *unchecked* (e.g. created from `MemoryAddress::ofLong`) it
> has no backing segment and can therefore not be dereferenced.  Because of this, one could argue that the signature of
> the `MemoryAddress::copy` is overly general, as it allows both *checked* and *unchecked* addresses, while only clearly
> making sense for the former.  A way out would be again, to express `copy` in terms of segment - and, as John points
> out, the starting point could be to look at `System::arraycopy` - e.g. assuming that `MemorySegment` ~= array:
> static void copy(MemorySegment src, long srcOffset, MemorySegment dst, long dstOffset, long length)
> 
> This is all and well; now we can only pass segments to the routine, which rules out the *unchecked* address issue. But
> still feels a bit redundant - after all, we can easily express offsets in terms of segment slices (which was pointed
> out by Paul at some point). So we can actually do this:  static void copy(MemorySegment src, MemorySegment dst)
> 
> where `dst` is assumed to be bigger (or equal) than `src`, where both starting offsets are assumed to be zero and where
> the length of the copy is assumed to be the length of the `src` segment. Since now the signature is expressed in terms
> of segment, it makes sense to move the method under `MemorySegment` too.
> This is nice, but I think that once we view this method as operating on segments, I think it makes sense to turn them
> into proper instance methods, and just do:
> void copyTo(MemorySegment dst)
> Can we do the same with `fill`? I think so:
> 
> void fill(byte value)
> Very simple (instance) method.
> 
> And, looking a bit ahead, we will want to have a functionality equivalent to `Arrays.mismatch` - which we can also
> express as an instance method:
> int mismatch(MemorySegment that)
> 
> So my recommendation would be for:
> 
> 1. moving all methods under `MemorySegment`
> 2. express all signatures in terms of `MemorySegment`
> 3. make them instance methods (rather than static)
> 
> This should lead to a more self-consistent, and discoverable API. Thoughts?

@mcimadamore I really like this suggestion (instance methods on MemorySegment). Especially given unchecked memory
addresses, your proposal to move `copy` to `MemorySegment`, will result in the API point being more obviuosly correct
by design ( less potential to use incorrectly ). And as you mention, we probably want to get this right now, since
`mismatch` is next on my todo list ;-) ( And mismatch will not work on unchecked addresses )

This design assumes slices are relatively cheap, given that more slicing will be needed to interact with API points,
but that should probably be ok.

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

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