RFR: 8334223: Make Arena MEMFLAGs immutable [v2]

[Thomas Stuefe]

On Tue, 18 Jun 2024 09:36:12 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - feedback johan
>>  - Merge branch 'master' into arena-constify-memflags
>>  - start
>
> src/hotspot/share/runtime/javaThread.hpp line 1138:
> 
>> 1136:   bool is_attaching_via_jni() const { return _jni_attach_state == _attaching_via_jni; }
>> 1137:   bool has_attached_via_jni() const { return is_attaching_via_jni() || _jni_attach_state == _attached_via_jni; }
>> 1138:   inline void set_is_attaching_via_jni();
> 
> So the only thing I don't like about this aspect is that this has to be called at construction time, so having it be a stand-alone function call invites misuse. I'm tempted to add a factory method:
> 
> JavaThread* JavaThread::new_attaching_thread() {
>   JavaThread* jt = new JavaThread();
>   jt>set_is_attaching_via_jni();
> }
> 
> and make `set_is_attaching_via_jni()` private.
> 
> What do you think?

Yes, I like that better, too. Okay, I'll do that.

> src/hotspot/share/runtime/javaThread.inline.hpp line 201:
> 
>> 199: inline void JavaThread::set_is_attaching_via_jni() {
>> 200:   _jni_attach_state = _attaching_via_jni;
>> 201:   OrderAccess::fence();
> 
> No need for a fence here as this should be set before the thread has been "published" and is visible to anyone else. In contrast `set_done_attaching_via_jni` is set after the thread has been published.

I had the vague notion of not wanting to tie `set_done_attaching_via_jni` to an exact point in time. But that was not well thought out anyway. With a factory method, this problem disappears.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19693#discussion_r1644897175
PR Review Comment: https://git.openjdk.org/jdk/pull/19693#discussion_r1644899449