[foreign-memaccess] RFR 8224483: Split MemoryAddress into separate address/region abstractions

[Maurizio Cimadamore]

Addressed all comments and pushed

Thanks
Maurizio

On 21/05/2019 18:50, Maurizio Cimadamore wrote:
>
> On 21/05/2019 17:51, Jorn Vernee wrote:
>> Hi,
>>
>> Some comments:
>>
>> MemoryAddress.java
>> - I'm wondering if it makes sense to move the ofByteBuffer method to 
>> MemorySegment? (This would also mirror the internal impl)
> Was thinking that too - probably it makes sense, yes.
>>
>> MemoryAddressImpl.java
>> - `((MemoryScopeImpl) segment().scope()).checkAlive()` is used in a 
>> couple of places. Maybe move this to MemorySegmentImpl, and then call 
>> segment.checkAlive()?
> ok
>> - I think asDirectByteBuffer needs to do a liveness check as well 
>> (should probably just call checkAccess?).
> yep - this part is untested as of yet (as we need to do more 
> validation) but I agree.
>>
>> MemorySegmentImpl.java
>> - OfEverything is unused and could be removed at this point.
> I thought I did... probably forgot to do the last bit :-)
>> - `resize` needs to check for negative offset as well, since we're 
>> coming directly from user code. I think this check could be replaced 
>> with a call to checkRange (adding a < 0 check for the length there).
>
> Good point.
>
>
> Thanks
> Maurizio
>
>>
>> Cheers,
>> Jorn
>>
>> Maurizio Cimadamore schreef op 2019-05-21 13:52:
>>> Following discussion at [1], I decided to go ahead and split
>>> MemoryAddres into two abstractions:
>>>
>>> * MemroyAddress will now embody an offest into a...
>>> * MemorySegment, which represent a contiguous region of memory
>>>
>>> The name MemorySegment came from an internal discussion with Brian,
>>> where he pointed out that MemoryScope and MemoryRegion where a bit too
>>> overlapping, in the english sense of the word. MemorySegment suggests
>>> something that has lower/upper boundaries.
>>>
>>> Webrev:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/8224483/
>>>
>>> There are many changes since the last patch discussed - mostly
>>> because, as I was adding the lower-level allocate methods in
>>> MemoryScope I realized that we were missing a lot of checks (w.r.t.
>>> alignment and sizes). I've added those and added tests for these as
>>> well (2 new tests have been added). Also, the Scope::allocate API
>>> should reflect the fact that Unsafe might fail to allocate - again
>>> added a test for this.
>>>
>>> Maurizio