> 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 > > _Update_ > --- > Switched too using a RAII object to track the context instead of putting code > in a closure. But all the encapsulation is still the same. > > Testing: Tier 1-7 > > _Update_ > --- >> @stefank suggested splitting out unloading klass logic change into a >> separate issue [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718). >> >> Will probably also limit this PR to only encapsulation. (Skipping the linked >> list optimisation) And create a separate issue for that as well. >> >> But this creates a chain of three dependent issues. >> [JDK-8291237](https://bugs.openjdk.org/browse/JDK-8291237) depends on >> [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718). And the link >> list optimisation depend will depend on >> [JDK-8291237](https://bugs.openjdk.org/browse/JDK-8291237). >> >> Will mark this as a draft for now and create a PR for >> [JDK-8291718](https://bugs.openjdk.org/browse/JDK-8291718) first. > > _Update_ > --- > Testing after 11d9dd2: Oracle platforms tier 1-5
Axel Boldt-Christmas has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 29 commits: - Merge remote-tracking branch 'upstream/master' into JDK-8291237 - Add asserts and comments - Merge remote-tracking branch 'upstream/master' into JDK-8291237 - Add context active assert - Cleanup comment - Add list optimization - Merge remote-tracking branch 'upstream/master' into JDK-8291237 - Rename deoptimize_done enum value - Add missing code from list revert - Initial draft new terminology - ... and 19 more: https://git.openjdk.org/jdk/compare/512fee1d...c35f5ed6 ------------- Changes: https://git.openjdk.org/jdk/pull/9655/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9655&range=08 Stats: 753 lines in 27 files changed: 351 ins; 282 del; 120 mod Patch: https://git.openjdk.org/jdk/pull/9655.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9655/head:pull/9655 PR: https://git.openjdk.org/jdk/pull/9655