Suggested fix for JDK-4724038 (Add unmap method to MappedByteBuffer)

[Vitaly Davidovich]

There's still a race condition since someone may have passed the guard,
entered a safepoint, and then proceeded to access the memory.

sent from my phone
On Sep 10, 2015 6:29 PM, "Peter Levart" <peter.levart at gmail.com> wrote:

> Hi guys,
>
> Perhaps there's no need for this protection/trap dance. If the situation
> is never tripped in correct programs (that unmap only after noone is using
> the buffers any more), then checking for address and throwing in case it is
> equal to some guard value is a never taken branch that is predicted
> perfectly. I wrote this little benchmark to test this claim:
>
> @BenchmarkMode(Mode.AverageTime)
> @Fork(value = 1, warmups = 0)
> @Warmup(iterations = 5)
> @Measurement(iterations = 10)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @State(Scope.Benchmark)
> public class MappedBufferBench {
>
>     private ByteBuffer bb;
>
>     @Setup(Level.Trial)
>     public void setup() {
>         bb = ByteBuffer.allocateDirect(64);
>     }
>
>     @Benchmark
>     public int directBufferGet() {
>         int sum = 0;
>         for (int i = 0; i < 64; i++) {
>             sum += bb.get(i);
>         }
>         return sum;
>     }
> }
>
>
> The results are:
>
> Original:
>
> Benchmark                                 Mode   Samples Score  Score
> error    Units
> j.t.MappedBufferBench.directBufferGet     avgt        10 17.740
> 0.247    ns/op
>
> Patched:
>
> Benchmark                                 Mode   Samples Score  Score
> error    Units
> j.t.MappedBufferBench.directBufferGet     avgt        10 17.796
> 0.220    ns/op
>
>
>
> What did I patch? There's a private method in DirectByteBuffer to convert
> index to address:
>
> Original:
>
>     private long ix(int i) {
>         return address + (i << 0);
>     }
>
> Patched:
>
>     private long ix(int i) {
>         long a = address;
>         if (a == 0L) throw new IllegalStateException();
>         return a + (i << 0);
>     }
>
>
>
> That's not all that has to be done of course. There would still have to be
> a wait for safe-point to return before unmapping. This is just a
> demonstration that maybe guarding mapping with protection is not needed.
>
>
> Regards, Peter
>
> On 09/10/2015 04:37 PM, David M. Lloyd wrote:
>
>> Or, the Java methods which wrap this access can just catch NPE and throw
>> the new exception type.
>>
>> On 09/10/2015 09:35 AM, Vitaly Davidovich wrote:
>>
>>> Well, you'd probably want something other than NPE here -- perhaps a new
>>> dedicated exception to signal this condition.  And this means the
>>> segfault
>>> handling now needs to know about this type of situation as well, rather
>>> than just NPEs.
>>>
>>> On Thu, Sep 10, 2015 at 10:32 AM, Andrew Haley <aph at redhat.com> wrote:
>>>
>>> On 09/10/2015 03:26 PM, Vitaly Davidovich wrote:
>>>>
>>>>> Yes, so what happens when that guard page is accessed by a thread after
>>>>>
>>>> safepoint?
>>>>
>>>> A segfault and a null pointer exception.
>>>>
>>>> Andrew.
>>>>
>>>>
>>>>
>>
>