[foreign-memaccess] [Rev 01] RFR: Move "owner" field and thread-confinement checks to MemoryScope
mcimadamore at openjdk.java.net
Fri May 15 11:50:26 UTC 2020
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 ?
More information about the panama-dev