RFR: 8299312: Clean up BarrierSetNMethod [v2]

Fei Yang fyang at openjdk.org
Sat Jan 7 10:11:53 UTC 2023


On Wed, 4 Jan 2023 14:50:20 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> The terminology in BarrierSetNMethod is not crisp. In platform code we talk about a per-nmethod "guard value", but on shared level we call the same value arm value or disarm value in different contexts. But it really depends on the value whether the nmethod is disarmed or armed. We should embrace the "guard value" terminology and lift it in to the shared code level.
>> We also have more functionality than we need on platform level. The platform level only needs to know how to deoptimize, and how to set/get the guard value of an nmethod. The more specific functionality should be moved to the shared code and be expressed in terms of said setter/getter.
>
> Erik Österlund 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 four additional commits since the last revision:
> 
>  - ARM support
>  - Merge branch 'master' into 8299312_barrier_set_nmethod_cleanup
>  - Fix Shenandoah build
>  - 8299312: Clean up BarrierSetNMethod

Looks good to me.

src/hotspot/share/runtime/thread.hpp line 118:

> 116:   // On AArch64, the high order 32 bits are used by a "patching epoch" number
> 117:   // which reflects if this thread has executed the required fences, after
> 118:   // an nmethod gets disarmed. The low order 32 bit denote the disarmed value.

Nit:
I think this should be:
"The low order 32 bits denote the disarmed value."
instead of:
"The low order 32 bit denote the disarmed value."

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

Marked as reviewed by fyang (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11774


More information about the shenandoah-dev mailing list