[jdk17u-dev] RFR: 8300926: Several startup regressions ~6-70% in 21-b6 all platforms [v3]

[Paul Hohensee]

On Mon, 17 Apr 2023 14:08:35 GMT, Alexey Pavlyutkin <duke at openjdk.org> wrote:

>> Hi, here is backport of [JDK-8300926](https://bugs.openjdk.org/browse/JDK-8300926). The patch splits deoptimization routine into two separate phases: the first implies only marking methods to be deoptimized, and the second is proper deoptimization. Only the first one is run under taken Compile_lock. Without the patch the lock stays taken during whole deoptimization routine and that prevents compilations from other threads.
>> 
>> Original patch is applied with the following changes:
>> 
>> - changes to `void LambdaFormInvokers::regenerate_class(char* class_name, ClassFileStream& st, TRAPS)` applied to `void LambdaFormInvokers::reload_class(char* name, ClassFileStream& st, TRAPS)`
>> - `SystemDictionary::add_to_hierarchy(JavaThread* current, InstanceKlass* k)`
>> changed to `SystemDictionary::add_to_hierarchy(Thread* current, InstanceKlass* k)`,
>> cuz in 17 it's also called from
>> `SystemDictionaryShared::add_unregistered_class(Thread* current, InstanceKlass* k)`
>> - `SystemDictionary::add_to_hierarchy(JavaThread* current, InstanceKlass* k)` does not lock/unlock k->init_monitor()
>> - changes to `void CodeCache::make_marked_nmethods_deoptimized()` skipped
>> - removal of absent `void CodeCache::make_nmethod_deoptimized(CompiledMethod* nm)` ignored
>> - substitution of `CompiledMethod::MarkForDeoptimizationStatus` with `DeoptimizationStatus` and related changes are skipped because in 17 `_mark_for_deoptimization_status` always accessed under CompiledMethod_lock;
>> - changes to `void DependencyContext::mark_dependent_nmethods(DeoptimizationScope* deopt_scope, DepChange& changes)` applied with respect to the difference in baselines
>> - `void DependencyContext::remove_and_mark_for_deoptimization_all_dependents(DeoptimizationScope* deopt_scope)` completely ported with respect to baseline
>> - changes to nmethod related to Virtual Threads were skipped
>> - changes to `void InstanceKlass::initialize_impl(TRAPS)` applied to `bool InstanceKlass::link_class_impl(TRAPS)`
>> 
>> Regression: hotspot_all (amd64/20.04LTS) - no new failures +  `runtime/cds/appcds/dynamicArchive/LambdaProxyDuringShutdown.java` previously failing now passes
>
> Alexey Pavlyutkin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   removing trailing whitespaces

I'm not an expert in this area, but have a few comments.

Why not lock/unlock k->init_monitor in systemdictionary.cpp? UseVtableBasedCHA exists in 17u.

In codeCache.cpp, the ResourceMark at line 1185 is kept in the PR, but removed in the original commit.

Perhaps run tests with -XX:+DeoptimizeALot?

Looks like [JDK-8299074](https://bugs.openjdk.org/browse/JDK-8299074) is included.

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

PR Review: https://git.openjdk.org/jdk17u-dev/pull/1243#pullrequestreview-1494027030