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

[Paul Sandoz]

On Thu, 14 May 2020 14:16:04 GMT, Chris Hegarty <chegar at openjdk.org> wrote:

>> 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.

Shifting over `copy` to an instance method on `MemorySegment` so it can be with its friend `fill`, is i think the right
move. Other friends can join later.

For arrays we never really had any choice (static vs. instance), although i still hold out hope someday we can make
arrays implement a richer array'ish interface.

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

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