On Wed, 10 Feb 2021 07:34:59 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:

>> `JVM_LatestUserDefinedLoader` is called normally from 
>> `ObjectInputStream.resolveClass` -> `VM.latestUserDefinedLoader0`. And it 
>> takes a measurable time to walk the stack. There is JDK-8173368 that wants 
>> to replace it with `StackWalker`, but we can tune up the 
>> `JVM_LatestUserDefinedLoader` itself without changing the semantics of it 
>> (thus providing the backportability, including the releases that do not have 
>> `StackWalker`) and improving performance (thus providing a more aggressive 
>> baseline for `StackWalker` rewrite).
>> 
>> The key is to recognize that out of two checks: 1) checking for two special 
>> subclasses; 2) checking for user classloader -- the first one usually 
>> passes, and second one fails much more frequently. First check also requires 
>> traversing the superclasses upwards looking for match. Reversing the order 
>> of the checks, plus inlining the helper method improves performance without 
>> changing the semantics.
>> 
>> Out of curiosity, my previous patch dropped the first check completely, 
>> replacing it by asserts, and we definitely run into situation where that 
>> check is needed on some tests.
>> 
>> On my machine, `VM.latestUserDefinedLoader` invocation time drops from 115 
>> to 100 ns/op. Single-threaded SPECjvm2008:serial improves about 3% with this 
>> patch.
>> 
>> Additional testing:
>>  - [x] Ad-hoc benchmarks
>>  - [x] Linux x86_64 fastdebug, `tier1`, `tier2`, `tier3` 
>> 
>> ---------
>> ### Progress
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> - [ ] Change must be properly reviewed
>> 
>> 
>> 
>> ### Download
>> `$ git fetch https://git.openjdk.java.net/jdk pull/2485/head:pull/2485`
>> `$ git checkout pull/2485`
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Added a comment

Marked as reviewed by alanb (Reviewer).

src/hotspot/share/prims/jvm.cpp line 3297:

> 3295:           
> !ik->is_subclass_of(vmClasses::reflect_ConstructorAccessorImpl_klass())) {
> 3296:         return JNIHandles::make_local(THREAD, loader);
> 3297:       }

This okay looks but surprised it has a measurable (or micro) difference.

There has been several proposals over the years to improve 
latestUserDefinedLoader in the common case that OIS.resolveClass is not 
overridden. It may need to be looked at again. Ideally 
JVM_LastestUSerDefinedLoader would go away and there would be a solution based 
on StackWalker (but work would be required there to match the current 
performance).

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

PR: https://git.openjdk.java.net/jdk/pull/2485

Reply via email to