RFR: 8358821: patch_verified_entry causes problems, use nmethod entry barriers instead [v9]

[Martin Doerr]

On Wed, 25 Jun 2025 09:49:25 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Dean Long has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into 8358821-patch-verified-entry
>>  - 2nd try at arm fix
>>  - rename arm_with to guard_with
>>  - arm32 fix
>>  - s390 fix courtesy of Amit Kumar
>>  - remove is_sigill_not_entrant
>>  - more cleanup
>>  - more TheRealMDoerr suggestions
>>  - TheRealMDoerr suggestions
>>  - remove trailing space
>>  - ... and 6 more: https://git.openjdk.org/jdk/compare/6df0f5e3...a39c458c
>
> src/hotspot/share/gc/shared/barrierSetNMethod.hpp line 52:
> 
>> 50: 
>> 51: public:
>> 52:   BarrierSetNMethod() : _current_phase(initial) {}
> 
> @fisk: The initial value doesn't match our initialization in the nmethod entry barrier code were we use 0. That causes all new nmethods to run through the barrier code when they are called for the first time. I think that is unnecessary and it slows down the startup a bit. All oops should already be correct after the nmethod got installed. And for ZGC, we call `nmethod_patch_barriers` in `ZNMethod::register_nmethod`. So, I don't see any need to execute the barrier code. We could change the initialization to use `_current_phase`. Do you agree? Should I file a new JBS issue?

In other words, I think `disarm(nm);` is missing for some GCs. ZGC has it in `ZNMethod::register_nmethod`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25764#discussion_r2166322229