Make java.nio.ByteBuffer a sealed class

Sebastian Stenzel sebastian.stenzel at gmail.com
Mon Jul 26 16:07:42 UTC 2021


I see, then I will add some asserts for now and remove the dead code and leave ByteBuffers untouched. :-)

> Am 26.07.2021 um 17:54 schrieb Paul Sandoz <paul.sandoz at oracle.com>:
> 
> I understand the motivation, it's well intentioned, but I think there are better longer term approaches to reduce the technical debt by exploring use of the memory access and foreign API.
> 
>> 
> It may be that with method patterns (something not yet proposed in a JEP) we could embed the knowledge and match, in totality, using ByteBuffer.isDirect(). Something to explore if/when such a pattern feature is available.
> 
> Paul.
> 
> 
>> On Jul 23, 2021, at 2:48 PM, Sebastian Stenzel <sebastian.stenzel at gmail.com> wrote:
>> 
>> Sure, we can just assert that any ByteBuffer that isn't direct must be a HeapByteBuffer. But this assertion is based on our knowlege. The point is, that we could use a mechanism which would instead cause a compile time error if this ever changes. In my opinion this is the stronger tool to ensure correctness than relying on every contributor to know that only these existing implementations of ByteBuffers are allowed.
>> 
>> This change has no direct functional effect, it merely helps to reduce branches that are technically dead code and just exist to satisfy ByteBuffer's contract. In other words: There is no functional advantage. This is about reducing technical debt.
>> 
>>>> Am 23.07.2021 um 17:46 schrieb Paul Sandoz <paul.sandoz at oracle.com>:
>>> 
>>> I don’t see much advantage to this. Most of the (byte) buffer hierarchy is package private. Further, the hierarchy is set up for convenience: DirectByteBuffer extends MappedByteBuffer [*], even though not all DirectByteBuffer instances are mapped.
>>> 
>>> Internally we can assume if a ByteBuffer instance is also an instance sun.nio.ch.DirectBuffer (which in turn implies that ByteBuffer.isDirect() == true) then it’s a direct buffer. Otherwise, its a heap buffer and if hasArray() == true then the underlying array is accessible.
>>> 
>>> In the referenced code there is a precondition that the byte buffer is not read-only. Therefore, we can assume for a heap buffer the src array is accessible (use an assert if you wish).
>>> 
>>> My hope is this kind of code can improve when we can use MemorySegment.
>>> 
>>> Paul.
>>> 
>>> [*]
>>> public abstract class MappedByteBuffer
>>>  extends ByteBuffer
>>> {
>>> 
>>>  // This is a little bit backwards: By rights MappedByteBuffer should be a
>>>  // subclass of DirectByteBuffer, but to keep the spec clear and simple, and
>>>  // for optimization purposes, it's easier to do it the other way around.
>>>  // This works because DirectByteBuffer is a package-private class.
>>> 
>>>> On Jul 23, 2021, at 3:59 AM, Sebastian Stenzel <sebastian.stenzel at gmail.com> wrote:
>>>> 
>>>> Any thoughts on this?
>>>> 
>>>>>> On 8. Jul 2021, at 16:54, Sebastian Stenzel <sebastian.stenzel at gmail.com> wrote:
>>>>> 
>>>>> A while back, when working on JDK-8264110, we had a discussion about the nature of ByteBuffers, where I asked whether we could eventually make this a sealed class. [1]
>>>>> 
>>>>> The idea is, that basically there are only two kinds of ByteBuffers:
>>>>> - HeapByteBuffers
>>>>> - MappedByteBuffer → DirectByteBuffer
>>>>> 
>>>>> While the former is backed by an array, the latter isn't, which leads to branching such as `if (buffer.hasArray())`, which could be simplified, if we knew for sure to deal with either of the two kinds.
>>>>> 
>>>>> Now, that sealed classes are stable with JDK 17, I'd like to ask for your approval to make ByteBuffer a sealed class, so we can rely on an exhaustive list of ByteBuffer "flavours" at compile time.
>>>>> 
>>>>> This is a requirement for follow-up tasks, I'd like to work on, such as JDK-8264341.
>>>>> 
>>>>> 
>>>>> [1] https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/3217*discussion_r602528233__;Iw!!ACWV5N9M2RV99hQ!d_JnBuyAJIsQrELaJHP-M0nHv3V2m3DSmNuEbXwf9IUGjQhCS_2YioIA2Vs5gTysvA$ 
>>>> 
>>> 
> 


More information about the nio-dev mailing list