RFR 8191416: (bf) DirectByteBuffer extends MappedByteBuffer, confuses instanceof tests

[Remi Forax]

----- Mail original -----
> De: "Alan Bateman" <Alan.Bateman at oracle.com>
> À: "Brian Burkhalter" <brian.burkhalter at oracle.com>, "nio-dev" <nio-dev at openjdk.java.net>
> Envoyé: Mardi 6 Février 2018 17:41:23
> Objet: Re: RFR 8191416: (bf) DirectByteBuffer extends MappedByteBuffer, confuses instanceof tests

> On 06/02/2018 01:17, Brian Burkhalter wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8191416
>> http://cr.openjdk.java.net/~bpb/8191416/webrev.00/
>>
>> As suggested in the issue description, change force(), load(), and isLoaded() to
>> be no-ops when there is no backing file instead of throwing
>> UnsupportedOperationException.
>>
> It's an undocumented implementation detail that a direct buffer is a
> MappedByteBuffer. This dates back to the original implementation, is
> complicated to change, and maybe we should put the effort re-visiting
> it.  I read this patch as a short term mitigation for those that test if
> a buffer is a MappedByteBuffer so they can call force. I guess it's okay
> although I do wonder if this mitigation should be limited to force and
> don't touch loaded/isLoaded.

Revisiting this hack without introducing a perf penalty is not obvious, with a DirectByteBuffer being a MappedByteBuffer and if you do not use read-only buffers, DirectByteBuffer is the only implementation that will be loaded, so the JIT will not insert any typechecks before invoking a method on a ByteBuffer thanks to the class hierarchy analysis optimization.    

> 
> Minor comment is that the other methods uses "mapping" in the name so
> maybe hasFileMapping might be better.

or remove the method hasMappedFile and inline the body in all the calling methods given it's now a simple == test. 

> 
> -Alan

Rémi