[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8291237: Encapsulate nmethod Deoptimization logic
From:       Axel Boldt-Christmas <duke () openjdk ! org>
Date:       2022-07-28 15:16:42
Message-ID: petM7LbgbkT2-Xv68QGfkcTwUo3fggeiJ-jgx1_KD6w=.bf55ccd5-b478-48b9-95f3-daa96c809a94 () github ! com
[Download RAW message or body]

On Wed, 27 Jul 2022 12:55:04 GMT, Axel Boldt-Christmas <duke@openjdk.org> wrote:

> The proposal is to encapsulate the nmethod mark for deoptimization logic in one \
> place and only allow access to the `mark_for_deoptimization` from a closure object: \
> ```C++ class DeoptimizationMarkerClosure : StackObj {
> public:
> virtual void marker_do(Deoptimization::MarkFn mark_fn) = 0;
> };
> 
> This closure takes a `MarkFn` which it uses to mark which nmethods should be \
> deoptimized. This marking can only be done through the `MarkFn` and a `MarkFn` can \
> only be created in the following code which runs the closure.  ```C++
> {
> NoSafepointVerifier nsv;
> assert_locked_or_safepoint(Compile_lock);
> marker_closure.marker_do(MarkFn());
> anything_deoptimized = deoptimize_all_marked();
> }
> if (anything_deoptimized) {
> run_deoptimize_closure();
> }
> 
> This ensures that this logic is encapsulated and the `NoSafepointVerifier` and \
> `assert_locked_or_safepoint(Compile_lock)` makes `deoptimize_all_marked` not having \
> to scan the whole code cache sound. 
> The exception to this pattern, from `InstanceKlass::unload_class`, is discussed in \
> the JBS issue, and gives reasons why not marking for deoptimization there is ok. 
> An effect of this encapsulation is that the deoptimization logic was moved from the \
> `CodeCache` class to the `Deoptimization` class and the class redefinition logic \
> was moved from the `CodeCache` class to the `VM_RedefineClasses` class/operation.  
> Testing: Tier 1-5

@fisk suggested using a RAII context object instead of a closure to guarantee the \
encapsulated invariants. Testing the patch now and will push when done. Not having to \
create a closure everywhere makes the code less verbose and more readable.

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

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


[prev in list] [next in list] [prev in thread] [next in thread]