Ping: RFR: JDK-8205523: Explicit barriers for interpreter

[Erik Österlund]

Hi Roman,

I gotta say I like this a lot better. With the safe defaults for the 
non-expert, yet allowing you guys to be more precise where you know it 
is okay, I am okay with this.
Just a small thing though:

6290 void MacroAssembler::resolve(DecoratorSet decorators, Register obj) {
6291   // Use stronger ACCESS_WRITE by default.
6292   if ((decorators & ACCESS_READ) == 0) {
6293     decorators |= ACCESS_WRITE;
6294   }
6295   BarrierSetAssembler* bs = 
BarrierSet::barrier_set()->barrier_set_assembler();
6296   return bs->resolve(this, decorators, obj);
6297 }

Just in terms of semantics, I think you should really change these 
defaults to:

6292   if ((decorators & (ACCESS_READ | ACCESS_WRITE)) == 0) {
6293     decorators |= ACCESS_READ | ACCESS_WRITE;
6294   }

In other words, the default resolve unless specified is for ACCESS_READ 
| ACCESS_WRITE - you may read or write primitives on the resolved oop. I 
don't think it is necesarily intuitive that being allowed to write 
implicitly means that you are allowed to read too; that is just an 
implementation detail of relocating on writes as Shenandoah does things. 
So if you just change that, you have looks good from me. And I don't 
need another webrev for that.

Thanks,
/Erik

On 2018-07-02 13:56, Roman Kennke wrote:
>>> I am a fan of profile guided optimization. I would definitely not mind
>>> introducing these concepts in the compilers where they are with no doubt
>>> necessary (and we also have the right tools for dealing with this
>>> better). In fact, they already have read/write decorators that could be
>>> used for resolve barriers in our compilers, and can use algorithms to
>>> safely elide barriers where provably correct, so it makes perfect sense
>>> for me to use such concepts there.
>>> I'm just not sure that the interpreter needs to be polluted with this
>>> conceptual overhead, unless there is at least one benchmark that can
>>> show that we are solving an actual problem with this. Remember,
>>> premature optimizations are the root of all evil.
>> but efficient systems are made from thousands of tiny optimizations, each
>> one too small to be measured above the noise on its own.
>>
> After some offline discussion with Erik, I want to propose a solution
> that 1. keeps the API simple to use, even if not sure if it's a read- or
> write-access (defaulting to stronger write, 2. remains flexible enough
> to optimize for read-only acces.
>
> This changeset introduces an API BarrierSetAssembler::resolve() which
> takes the 'hint' about read- vs write-access via a Decorator ACCESS_READ
> and ACCESS_WRITE. The API frontend in MacroAssembler::resolve() sets
> ACCESS_WRITE if ACCESS_READ is not explicitely set.
>
> Incremental webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8205523/webrev.01.diff/
> Full webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8205523/webrev.01/
>
> If this proposal gets accepted, I'd take this further (in separate RFEs):
> 1. make C1 use ACCESS_READ and ACCESS_WRITE instead of C1_READ_ACCESS
> and C1_WRITE_ACCESS. Those uses are currently no good anyway (as they
> are currently passed into Access API that 'knows' what it is, e.g.
> store_at/load_at, etc).
> 2. introduce same pattern for resolve() in runtime (and later, c1 and c2).
>
> What do you think?
>
> Roman
>