RFR: 8306767: Concurrent repacking of extra data in MethodData is potentially unsafe [v16]

[Emanuel Peter]

On Thu, 11 Jan 2024 13:33:29 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed typo
>
> src/hotspot/share/runtime/deoptimization.cpp line 2406:
> 
>> 2404:         reprofile = true;
>> 2405:       }
>> 2406: 
> 
> Why is it safe to move this here?

Do you think it is not safe?
We have exactly the same conditions:
`make_not_entrant` and `pdata != nullptr` (the second can only hold if we take the current path which gets `pdata`, else it would be `nullptr` anyway).

The only question is with

      if (!nm->make_not_entrant()) {
        return; // the call did not change nmethod's state
      }


Hmm, maybe I need to copy that here too.But that's kinda ugly. Do you think that is necessary? Any other suggestion?

The idea was to limit the scope of `pdata`. And I kinda have to do that because only in this path do I have a guarantee that `trap_mdo != nullptr`. Otherwise I need to somehow make the lock conditional in the outer scope, that is even more nasty.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16840#discussion_r1449058393