On Thu, 16 Nov 2023 17:31:46 GMT, Paul Sandoz <psan...@openjdk.org> wrote:

>> See the JBS issue for an extended problem description.
>> 
>> This patch changes the specification and implementation of 
>> `MethodHandles::byteArrayViewVarHandle`, 
>> `MethodHandles::byteBufferViewVarHandle`, `ByteBuffer::alignedSlice`, and 
>> `ByteBuffer::alignmentOffset` to weaken the guarantees they make about the 
>> alignment of Java array elements, in order to bring them in line with the 
>> guarantees made by an arbitrary JVM implementation (which makes no 
>> guarantees about array element alignment beyond them being aligned to their 
>> natural alignment).
>> 
>> - `MethodHandles::byteArrayViewVarHandle`: we can not guarantee any 
>> alignment for the accesses. Which means we can only reliably support plain 
>> get and set access modes. The javadoc text explaining which other access 
>> modes are supported, or how to compute aligned offsets into the array is 
>> dropped, as it is not guaranteed to be correct on all JVM implementations. 
>> The implementation of the returned VarHandle is changed to throw an 
>> `UnsupportedOperationException` for the unsupported access modes, as 
>> mandated by the spec of `VarHandle` [1].
>> 
>> - `MethodHandles::byteBufferViewVarHandle`: the implementation/spec is 
>> incorrect when accessing a heap buffer (wrapping a byte[]), for the same 
>> reasons as `byteArrayViewVarHandle`. The spec is changed to specify that 
>> when accessing a _heap buffer_, only plain get and set access modes are 
>> supported. The implementation of the returned var handle is changed to throw 
>> an `IllegalStateException` when an access is attempted on a heap buffer 
>> using an access mode other than plain get or set. Note that we don't throw 
>> an outright `UnsupportedOperationException` for this case, since whether the 
>> access modes are supported depends on the byte buffer instance being used.
>> 
>> - `ByteBuffer::alignedSlice` and `ByteBuffer::alignmentOffset`: The former 
>> method depends directly on the latter for all its alignment computations. We 
>> change the implementation of the latter method to throw an 
>> `UnsupportedOperationException` for all unit sizes greater than 1, when the 
>> buffer is non-direct. This change is largely covered by the existing 
>> specification:
>> 
>> 
>>      * @throws UnsupportedOperationException
>>      *         If the native platform does not guarantee stable alignment 
>> offset
>>      *         values for the given unit size when managing the memory 
>> regions
>>      *         of buffers of the same kind as this buffer (direct or
>>      *         non-direct).  For example, if garbage collection would...
>
> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 4518:
> 
>> 4516:      * Only plain {@linkplain VarHandle.AccessMode#GET get} and 
>> {@linkplain VarHandle.AccessMode#SET set}
>> 4517:      * access modes are supported by the returned var handle. For all 
>> other access modes, an
>> 4518:      * {@link UnsupportedOperationException} will be thrown.
> 
> I recommend adding an api note explaining that native memory segments, direct 
> byte buffers, or heap memory segments backed by long[] should be used if 
> support for other access modes are required.

Good idea. Thanks

> src/java.base/share/classes/java/nio/X-Buffer.java.template line 2218:
> 
>> 2216:      * @implNote
>> 2217:      * This implementation throws {@code 
>> UnsupportedOperationException} for
>> 2218:      * non-direct buffers when the given unit size is greater then 
>> {@code 1}.
> 
> This is no longer an implementation note, its now part of the specified API. 
> So i think we can simplify the text of the `@throws UOE ...` to just say:
> 
> @throws UOE if the buffer is non-direct and the unit size > 1
> 
> Same for the other method.

Right. Good idea

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16681#discussion_r1396147281
PR Review Comment: https://git.openjdk.org/jdk/pull/16681#discussion_r1396147495

Reply via email to