RFR: JDK-8210187: Explicit barriers for C2

Per Liden per.liden at oracle.com
Fri Aug 31 11:36:44 UTC 2018


Hi Roman,

On 08/31/2018 12:51 AM, Roman Kennke wrote:
> Am 30.08.2018 um 20:27 schrieb Erik Osterlund:
>> Hi Alexey,
>>
>>> On 30 Aug 2018, at 19:02, Aleksey Shipilev <shade at redhat.com> wrote:
>>>
>>> On 08/30/2018 05:58 PM, Roman Kennke wrote:
>>>>>>  From a far away interface design perspective, those are still heap
>>>>> accesses and GCs might be interested in intercepting e.g. final values.
>>>>> Maybe keep the resolves there and augment them with something like
>>>>> ACCESS_FINAL and/or ACCESS_STABLE and let the backend decide what to do
>>>>> with it? For example, in Shenandoah we have an option to optimize for
>>>>> the final/stable stuff and exploit the UD in the JMM in our favour, or
>>>>> to be conservative in favour of crazy applications, that could react on
>>>>> such a decorator. WDYT?
>>>>>
>>>> If final and stable properties can be exploited by the GC, then
>>>> annotating that explicitly with decorators makes sense to me. However,
>>>> it seems more intuitive to me to make such annotations in the relevant
>>>> accesses, and not in a resolve barrier. Or what do you think?
>>>
>>> I think we need barriers by default, because we don't know what GC would do. There is no knowledge
>>> that it is safe to read the backing String arrays on this level. I agree that additional decorator
>>> tags about the nature of access may help GC to decide what to do with it. That feels like a good
>>> follow-up, which requires introducing resolve barriers anyway.
>>
>> I suppose that is true, despite it being a bit weird that we are cluttering the code with barrier hooks that 0 GCs (Shenandoah included) require. I suppose I am okay with that though.
>>
>>> I think current placement is beneficial, because otherwise we need to put barriers into the each of
>>> platform-specific assemblers that expand the relevant String intrinsic nodes. Which appears more
>>> tedious, and it makes barriers opaque for high-level optimizations, e.g. folding the consecutive GC
>>> barriers. (This seems to be the reason to have BarrierSetC2 to begin with!)
>>
>> Yeah I don’t think we want to go down that route either.
>>
>> Looks good. Ship it!
> 
> Thank you.
> 
> In off-list discussions, Roland pointed out that some of those
> resolve()s are only there to support object equality. Those should be
> treated differently, with separate API(s) in BarrierSetC2. I excluded
> those from the changeset:
> 
> Incremental:
> http://cr.openjdk.java.net/~rkennke/JDK-8210187/webrev.01.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8210187/webrev.01/

Just a quick request. Please try to follow the coding style of the 
class/file when you're changing it. For example in barrierSetC2.hpp I 
see you added this, which now sticks out as it doesn't follow the style 
of the surrounding functions.

+  virtual Node* resolve(GraphKit* kit, Node* n, DecoratorSet 
decorators) const {
+    return n;
+  }
+

So, please make it a one liner, like the other functions. I happen to 
personally prefer the multi-line style, but not when it goes against 
what every other function in the class looks like. Let's keep things 
consistent, at least within the same class/file. Thanks!

cheers,
Per

> 
> Still good?
> 
> I'd also like to have Roland's (or somebody else from C2 land) agreement.
> 
> Thanks,
> Roman
> 
> 
> 



More information about the hotspot-gc-dev mailing list