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

[Peter Levart]

On 09/11/2015 12:42 AM, Vitaly Davidovich wrote:
>
> There's still a race condition since someone may have passed the 
> guard, entered a safepoint, and then proceeded to access the memory.
>

Yeah, and I realized yesterday after posting that this wouldn't work for 
views of the buffer (dulicate(), slice(), asXxxBuffer()) since they copy 
over the address to their instance field, so multiple instances point to 
the same block of memory with their own address fields...

Regards, Peter

> sent from my phone
>
> On Sep 10, 2015 6:29 PM, "Peter Levart" <peter.levart at gmail.com 
> <mailto: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 <mailto: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.
>
>
>
>