[foreign-memaccess] [Rev 01] RFR: Move "owner" field and thread-confinement checks to MemoryScope

[Maurizio Cimadamore]

On Fri, 15 May 2020 09:41:51 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Peter Levart has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improvements to MemoryScope code considering Maurizio's comments.
>
> Overall, I like this and I agree that moving ownership in scope makes the code easier to follow. I've added some
> specific comments.

> So maybe just remove the comment?

No, leave it there - I'm fine with the explanation.

> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryScope.java line 95:
> 
>> 94:     private final Thread owner;
>> 95:     boolean closed; // = false
>> 96:     private static final VarHandle CLOSED;
> 
> Is there any real advantage in not having the initialization here? I mean, I get we probably save one putfield - but I
> would say that if the code is hot, C2 is probably able to detect that the store is useless? I kind of prefer the code
> to be self-evident rather then to use comments, where possible, of course. I've seen in the past cases where seemingly
> innocuous redundant initializer subtly changed happens-before order; but this shouldn't be the case here?

> So maybe just rename the parameter to newOwner ?

Yes please

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/167