[PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

[Paul Sandoz]

> On Feb 1, 2018, at 4:03 PM, Hans Boehm <hboehm at google.com> wrote:
> 
> 
> On Thu, Feb 1, 2018 at 2:50 PM, Paul Sandoz <paul.sandoz at oracle.com <mailto:paul.sandoz at oracle.com>> wrote:
> >
> > Hi Ben,
> >
> > I don’t think you require the fence in all the cases you have currently placed it e.g. here for example
> >
> >         $memtype$ y = $toBits$(x);
> >         UNSAFE.put$Memtype$Unaligned(null, a, y, bigEndian);
> > +        Reference.reachabilityFence(this);
> >         return this;
> >
> > since “this” is being returned it will be kept live during the unsafe access.
> 
> I don't see how that's guaranteed by the JLS, specifically 12.6.2. What if this function is inlined into the caller, and the caller drops the result?
> 

Ah yes, my apologies, i was thinking too narrowly. Your document reminds we should really use the try/finally pattern for any updates DBB code for reasons you suggest even if not strictly needed.


> I would still greatly prefer to see an added mechanism (possibly along the lines of https://docs.google.com/document/d/1yMC4VzZobMQTrVAraMy7xBdWPCJ0p7G5r_43yaqsj-s/edit?usp=sharing <https://docs.google.com/document/d/1yMC4VzZobMQTrVAraMy7xBdWPCJ0p7G5r_43yaqsj-s/edit?usp=sharing>) that associates the reachability requirements with fields or classes, rather than putting a reachabilityFence() in every method accessing a field. I'm sure there are cases where reachabilityFence() is the preferred mechanism. But for the code I've looked at recently, I haven't actually found one yet. I don't think it is a good mechanism here, either.
> 

IIRC we discussed such approaches before and concluded reachabilityFence was the most expedient way to get something functional into developers hands if not in the most ideal and expressive way for all cases.


> >
> > Would you mind providing a JMH benchmark and results for the performance with and without the fence for say a put and/or a get. I would like to understand the performance impact on HotSpot, this is one reason why we have yet to add such fences as it will likely impact performance.
> >
> > At the moment we are relying on the method not being inlined, which is an expedient technique to make it functional and keep a reference alive but not necessarily optimal for usages in DBB.
> 
> But that should be fixable, right?

I believe so, it comes down to prioritizing and expending some compiler engineering effort.

Thanks,
Paul.


> It shouldn't generate any code beyond potentially preventing register or stack slot reuse; the required effect of reachabilityFence (in a conventional implementation) is to ensure that the argument is considered live at any preceding GC points.
> 
> >
> > For more details see:
> >
> >   https://bugs.openjdk.java.net/browse/JDK-8169605 <https://bugs.openjdk.java.net/browse/JDK-8169605> <https://bugs.openjdk.java.net/browse/JDK-8169605 <https://bugs.openjdk.java.net/browse/JDK-8169605>>
> >   https://bugs.openjdk.java.net/browse/JDK-8149610 <https://bugs.openjdk.java.net/browse/JDK-8149610> <https://bugs.openjdk.java.net/browse/JDK-8149610 <https://bugs.openjdk.java.net/browse/JDK-8149610>>
> >
> > Thanks,
> > Paul.