On Tue, 16 Jul 2024 14:46:13 GMT, Jorn Vernee <[email protected]> wrote:
>> This PR limits the number of cases in which we deoptimize frames when
>> closing a shared Arena. The initial intent of this was to improve the
>> performance of shared arena closure in cases where a lot of threads are
>> accessing and closing shared arenas at the same time (see attached
>> benchmark), but unfortunately even disabling deoptimization altogether does
>> not have a great effect on that benchmark.
>>
>> Nevertheless, I think the extra logging/testing/benchmark code, and comments
>> I've written, together with reducing the number of cases where we deoptimize
>> (which makes it clearer exactly why we need to deoptimize in the first
>> place), will be useful going forward. So, I've a create this PR out of them.
>>
>> In this PR:
>> - Deoptimizing is now only done in cases where it's needed, instead of
>> always. Which is in cases where we are not inside an `@Scoped` method, but
>> are inside a compiled frame that has a scoped access somewhere inside of it.
>> - I've separated the stack walking code (`for_scope_method`) from the code
>> that checks for a reference to the arena being closed
>> (`is_accessing_session`), and added logging code to the former. That also
>> required changing vframe code to accept an `ouputStream*` rather than always
>> printing to `tty`.
>> - Added a new test (`TestConcurrentClose`), that tries to close many shared
>> arenas at the same time, in order to stress that use case.
>> - Added a new benchmark (`ConcurrentClose`), that stresses the cases where
>> many threads are accessing and closing shared arenas.
>>
>> I've done several benchmark runs with different amounts of threads. The
>> confined case stays much more consistent, while the shared cases balloons up
>> in time spent quickly when there are more than 4 threads:
>>
>>
>> Benchmark Threads Mode Cnt Score Error Units
>> ConcurrentClose.sharedAccess 32 avgt 10 9017.397 ± 202.870 us/op
>> ConcurrentClose.sharedAccess 24 avgt 10 5178.214 ± 164.922 us/op
>> ConcurrentClose.sharedAccess 16 avgt 10 2224.420 ± 165.754 us/op
>> ConcurrentClose.sharedAccess 8 avgt 10 593.828 ± 8.321 us/op
>> ConcurrentClose.sharedAccess 7 avgt 10 470.700 ± 22.511 us/op
>> ConcurrentClose.sharedAccess 6 avgt 10 386.697 ± 59.170 us/op
>> ConcurrentClose.sharedAccess 5 avgt 10 291.157 ± 7.023 us/op
>> ConcurrentClose.sharedAccess 4 avgt 10 209.178 ± 5.802 us/op
>> ConcurrentClose.sharedAccess 1 avgt 10 ...
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JVMCI support
src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethod.java
line 62:
> 60:
> 61: /**
> 62: * Returns true if this method has a {@code Scoped} annotation.
Can you please make this a qualified name:
`jdk.internal.misc.ScopedMemoryAccess.Scoped`.
That makes it easier for someone not familiar with the code base to find.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20158#discussion_r1679575238
