JDK-8236853 : Memory access API refinements
  • Type: CSR
  • Component: core-libs
  • Priority: P2
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 14
  • Submitted: 2020-01-09
  • Updated: 2020-01-14
  • Resolved: 2020-01-14
Related Reports
CSR :  
JDK-8235837 - Memory access API refinements
Relates :  
JDK-8234050 - Implementation of Memory Access API (Incubator)
Description
Summary
-------

This CSR proposes few changes to the memory access API, which are aimed at addressing some comments received during the review process, as well as to improve the usability of the API.

Problem
-------

During the review process, we kept track of some API issues (in the issue form which this CSR is derived). Some of the issues have to do with API naming inconsistencies (for instance, the fact that `MemroryAddress::offset()` and `MemoryAddress::offset(long)` have very similar names, yet perform radically different operations). Other issues cover some usability problems when working with the API, especially in conjunction with unbound sequence layouts, as there's no way in the API to tell as to whether a memory layout has a defined size.

Solution
--------

To address the aforementioned issues, first we propose the following two cosmetic changes:

 * rename `MemoryAddress::offset(long)` to `MemoryAddress::addOffset(long)`
 * replace the `MemorySegment::isAccessible` predicate with the `MemorySegment::ownerThread` accessor

And, to address the usability issues when working with layout with unspecified size, we propose to add the following methods:

* add a predicate `MemoryLayout::hasSize`, which allows client to tell whether `MemoryLayout::bitSize` will throw or not.
* add the method `SequenceLayout::withElementCount(long)`, to allow clients to obtain new sequence layouts with given new size (which is useful to turn an unbounded sequence layout into a bounded sequence layout)
* add a method `MemoryLayout::map(UnaryOperator<MemoryLayout>, PathElement...)` which allow clients to create a copy of the current layout where the element selected by the layout path passed as argument is replaced by a new layout
* add a method `MemoryLayout::select(PathElement...)` which allow clients to select a (possibly nested) layout at a given path in the current layout

Specification
-------------

A specdiff of the proposed changes is attached in this CSR request. A javadoc including the changes is also available here:

http://cr.openjdk.java.net/~mcimadamore/8235837_v3_javadoc
Comments
Moving to Approved. As a code review comment, in say ValueLayout and other classes where hasSize is always true I recommend something like /** * Returns true; value layouts always have a size. */ @Override public boolean hasSize() {...}
14-01-2020
specdiff and javadoc link updated
14-01-2020
Yes, please submit an updated specdiff reflecting the changes.
14-01-2020
On @implSpec, again MemoryLayout is not meant for overriding (and will be sealed as toplevel javadoc says). So, I don't see a lot of value for doing it here. On the last comment, Loom is zeroing on an approach were Thread represents both fibers and legacy threads. So I think naming is correct as per latest Loom knowledge. Regarding the other two points, yes, withElementCount should throw on negative sizes, and it could be a good idea to override hasSize in the classes were it cannot throw. If you want I can submit another specdiff with these changes.
14-01-2020
A few comments from my initial passes over the proposed changes: MemoryLayout.{map, select}, etc. better with @implSpec tags for behavior of default methods If the hasSize method unconditionally returns true for some classes, that should be indicated in the javadoc, perhaps in as in ValueLayout. Should SequenceLayout.withElementCount throw IAE for a negative (or non-positive) argument? MemorySegment.ownerThread: would this be better as ownerRunnable? This can be revisited in the future, vis a vis Loom and other efforts.
14-01-2020