RFR(XL) 8198698: Support Lambda proxy classes in dynamic CDS archive
Calvin Cheung
calvin.cheung at oracle.com
Wed Jun 3 19:34:10 UTC 2020
Hi Mandy,
Thanks for your review.
On 6/2/20 11:38 AM, Mandy Chung wrote:
> Hi Calvin,
>
> To recap an offline discussion, I think we should use JDK bootcycle
> build to test this feature. For example generating a boot JDK with
> archived lambda proxy classes for java.base and javac to build itself
> and also use it to run JDK tier 1-3 tests. This would serve a good
> test bed. I also suggest to experiment how JDK can benefit from this
> feature (e.g. any java.base and javac performance gain). The module
> system initialization had to avoid using lambdas at startup time and
> we could re-examine this again.
This RFE supports archiving the lambda proxy classes in a dynamic
archive. In order to exercise this feature during JDK build time, we
probably need to include the lambda proxy classes in the default CDS
archive. Or have another dynamic CDS archive on top of the default CDS
archive. The dynamic archive may initially contain the classes required
by javac to compile a java program, like a HelloWorld. I think this
should be explored as a separate RFE.
I can get some perf numbers on running javac on HelloWorld later.
>
> On 5/29/20 2:29 PM, Calvin Cheung wrote:
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8198698
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev.00/
>>
>
> Hidden classes have the following properties:
> 1. not discoverable by name. It's not registered in the system
> dictionary.
> 2. It can be a dynamic nest member of a nest host. For lambda proxy
> class, it has the same nest host as the caller class (same runtime
> package as the caller class)
> 3. strong or weak link with the class loader. a hidden class may be
> unloaded even its defining class loader is alive.
>
> For the archived lambda proxy classes, do you re-assign a new class
> name to the shared lambda proxy class?
No, I didn't assign a new class name to a shared lambda proxy class.
> I suspect not. It's very rare that it will conflict with the name of
> other hidden classes. It's an option to patch the name to indicate
> it's from shared archive if it helps diagnosability.
In the class load trace (-Xlog:class+load), one will see the following
if the lambda proxy class is loaded from a dynamic archive:
[0.428s][info ][class,load ]
BasicLambdaApp$$Lambda$1/0x0000000800bd5258 source: shared objects file
(top)
>
> My comments below are related to the above.
>
> 1374 InstanceKlass*
> SystemDictionary::load_shared_lambda_proxy_class(InstanceKlass* ik,
> 1375 Handle class_loader,
> 1376 Handle protection_domain,
> 1377 PackageEntry* pkg_entry,
> 1378 TRAPS) {
> 1379 InstanceKlass* shared_n_h =
> SystemDictionaryShared::get_shared_nest_host(ik);
> 1380 assert(shared_n_h->is_shared(), "nest host must be in CDS archive");
> 1381 Symbol* cn = shared_n_h->name();
> 1382 Klass *s = resolve_or_fail(cn, class_loader, protection_domain,
> true, CHECK_NULL);
> 1383 if (s != shared_n_h) {
> 1384 // The dynamically resolved nest_host is not the same as the one
> we used during dump time,
> 1385 // so we cannot use ik.
> 1386 return NULL;The above resolve_or_fail is to ensure that H is the
> same class loaded by the given class loader. Another validation that you should do is to check
> the nest host and nest member must be in the same runtime package -
> maybeik->set_nest_host(shared_n_h, THREAD) validates the runtime package??
I've added an assert:
1398 InstanceKlass* loaded_ik = load_shared_class(ik, class_loader,
protection_domain, NULL, pkg_entry, CHECK_NULL);
1399
1400 assert(shared_nest_host->is_same_class_package(ik),
1401 "lambda proxy class and its nest host must be in the same
package");
The assert needs to be after load_shared_class so that the class loader
and PackageEntry have been setup.
>
>
> SystemDictionaryShared::load_shared_lambda_proxy_class
>
> 1679 InstanceKlass* shared_n_h = get_shared_nest_host(lambda_ik);
> 1680 assert(shared_n_h != NULL, "unexpected NULL _nest_host");
> 1681
> 1682 InstanceKlass* loaded_lambda =
> 1683 SystemDictionary::load_shared_lambda_proxy_class(lambda_ik, class_loader, protection_domain, pkg_entry, CHECK_NULL);
> 1684
> 1685 InstanceKlass* n_h = caller_ik->nest_host(THREAD);
> 1686 assert(n_h->has_nest_member(caller_ik, THREAD) ||
> 1687 n_h == caller_ik, "mismatched nest host");
>
> I think you meant to check n_h->has_nest_member(loaded_lambda, THREAD).
I saw David has commented on this. So I'll leave the assert as before
and I've added another assert (see line 1691):
1687 // The following ensures that the caller's nest host is the same
as the lambda proxy's
1688 // nest host recorded at dump time.
1689 assert(nest_host->has_nest_member(caller_ik, THREAD) ||
1690 nest_host == caller_ik, "caller_ik failed nest member check");
1691 assert(nest_host == shared_nest_host, "mismatched nest host");
>
> It'd be good to add a comment to say that this ensures thatthe caller's nest host must be the same as the lambda proxy's nest
> host recorded at dump time.
Done.
>
> Nit: the variable name "n_h" or "_n_h" would be clearer if it's named
> "nest_host" or "_nest_host".
I've renamed n_h to nest_host.
>
> 1689 SystemDictionary::set_shared_class_init_state(lambda_ik, InstanceKlass::loaded);
It turns just setting the lambda_ik to the InstanceKlass::loaded state
is not enough. We need to do some of the stuff which are currently being
done in parse_stream. Please refer to the change in
SystemDictionary::load_shared_lambda_proxy_class.
>
> A lambda proxy class typically is eagerly initialized (unless a flag is given).
> Your change dumps the hidden class with no knowledge if `Lookup::defineHiddenClass`
> is called with "initialize" parameter is true or not. This may affect the runtime
> performance (for example MethodHandle will have class init barrier if the class
> is not yet initialized).
> This may have performance implication. I think we should consider
> passing the initialize parameter to LambdaProxyClassArchive::find such
> that if the class is found from CDS archive, it will invoke <clinit>
> before it returns to the library.
In InnerClassLambdaMetafactory, the !disableEagerInitialization will be
passed to LambdaProxyClassArchive.find which will pass down to vm.
271 Class<?> innerClass =
LambdaProxyClassArchive.find(targetClass,
272 samMethodName,
273 invokedType,
274 samMethodType,
275 implMethod,
276 instantiatedMethodType,
277 isSerializable,
278 markerInterfaces,
279 additionalBridges,
280 !disableEagerInitialization);
In SystemDictionary::load_shared_lambda_proxy_class, it checks the flag:
1422 if (initialize) {
1423 loaded_ik->initialize(CHECK_NULL);
1424 }
On a related note, in the existing jvm_lookup_define_class in jvm.cpp:
if (init) {
ik->initialize(CHECK_NULL);
} else {
ik->link_class(CHECK_NULL);
}
I don't think the else is necessary as the ik->link_class(CHECK_NULL)
has been done within the SystemDictionary::parse_stream.
> One other point I brought up - strong vs weak hidden class (i.e. is it
> using the primary CLD or the per-mirror CLD?). I have lost myself
> where you do this in the implementation. Can you point that out? Is it
> recorded in the shared archive and make sure it uses the exact CLD
> when it's loaded?
Currently, the strong vs weak hidden class isn't recorded in the archive.
Because in InnerClassLambdaMetafactory.generateInnerClass, there's the
following.
Lookup lookup = caller.defineHiddenClass(classBytes,
!disableEagerInitialization, NESTMATE, STRONG);
Also, the above line of code is after the call to
LambdaProxyClassArchive.register. (Refer to call stack below.)
1 - frame( sp=0x00007ffff7fc2730, unextended_sp=0x00007ffff7fc2730,
fp=0x00007ffff7fc27c8, pc=0x00007fffd8d3daca)
java.lang.ClassLoader.defineClass0(Native Method)
2 - frame( sp=0x00007ffff7fc27d8, unextended_sp=0x00007ffff7fc27e8,
fp=0x00007ffff7fc2880, pc=0x00007fffd8d34e00)
java.lang.System$2.defineClass(System.java:2193)
3 - frame( sp=0x00007ffff7fc2890, unextended_sp=0x00007ffff7fc2890,
fp=0x00007ffff7fc2920, pc=0x00007fffd8d34ee3)
java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClass(MethodHandles.java:2181)
4 - frame( sp=0x00007ffff7fc2930, unextended_sp=0x00007ffff7fc2950,
fp=0x00007ffff7fc29b0, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClassAsLookup(MethodHandles.java:2163)
5 - frame( sp=0x00007ffff7fc29c0, unextended_sp=0x00007ffff7fc29c8,
fp=0x00007ffff7fc2a20, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandles$Lookup.defineHiddenClass(MethodHandles.java:1961)
6 - frame( sp=0x00007ffff7fc2a30, unextended_sp=0x00007ffff7fc2a30,
fp=0x00007ffff7fc2a98, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.generateInnerClass(InnerClassLambdaMetafactory.java:379)
7 - frame( sp=0x00007ffff7fc2aa8, unextended_sp=0x00007ffff7fc2ae8,
fp=0x00007ffff7fc2b38, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:280)
8 - frame( sp=0x00007ffff7fc2b48, unextended_sp=0x00007ffff7fc2b50,
fp=0x00007ffff7fc2ba0, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:209)
9 - frame( sp=0x00007ffff7fc2bb0, unextended_sp=0x00007ffff7fc2bc8,
fp=0x00007ffff7fc2c18, pc=0x00007fffd8d34e00)
java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:328)
10 - frame( sp=0x00007ffff7fc2c28, unextended_sp=0x00007ffff7fc2c28,
fp=0x00007ffff7fc2ca8, pc=0x00007fffd8d34e00)
java.lang.invoke.DirectMethodHandle$Holder.invokeStatic
11 - frame( sp=0x00007ffff7fc2cb8, unextended_sp=0x00007ffff7fc2cc0,
fp=0x00007ffff7fc2d40, pc=0x00007fffd8d34e00)
java.lang.invoke.Invokers$Holder.invokeExact_MT
12 - frame( sp=0x00007ffff7fc2d50, unextended_sp=0x00007ffff7fc2d50,
fp=0x00007ffff7fc2dd8, pc=0x00007fffd8d34e00)
java.lang.invoke.BootstrapMethodInvoker.invoke(BootstrapMethodInvoker.java:127)
13 - frame( sp=0x00007ffff7fc2de8, unextended_sp=0x00007ffff7fc2e30,
fp=0x00007ffff7fc2ea8, pc=0x00007fffd8d34e00)
java.lang.invoke.CallSite.makeSite(CallSite.java:315)
14 - frame( sp=0x00007ffff7fc2eb8, unextended_sp=0x00007ffff7fc2ec8,
fp=0x00007ffff7fc2f38, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:270)
15 - frame( sp=0x00007ffff7fc2f48, unextended_sp=0x00007ffff7fc2f50,
fp=0x00007ffff7fc2fc8, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:260)
C frame (sp=0x00007ffff7fc2fd8 unextended sp=0x00007ffff7fc2ff8,
fp=0x00007ffff7fc3090, real_fp=0x00007ffff7fc3090, pc=0x00007fffd8d2b9ca)
(~Stub::call_stub)
BufferBlob (0x00007fffd8d2b810) used for StubRoutines (1)
16 - frame( sp=0x00007ffff7fc3880, unextended_sp=0x00007ffff7fc3880,
fp=0x00007ffff7fc38c8, pc=0x00007fffd8d542d3)
LambHello.main(LambHello.java:3)
-----
For now, I've added an assert in
JVM_RegisterLambdaProxyClassForArchiving to make sure the hidden class
is strong so that when things changed later, we'll notice it.
I also added a check in
SystemDictionaryShared::is_registered_lambda_proxy_class to make sure
the hidden class is strong.
As for the CLD, I didn't do anything special. In
SystemDictionary::load_shared_class:
1470 ClassLoaderData* loader_data =
ClassLoaderData::class_loader_data(class_loader());
It is consistent with what is being done in parse_stream for a strong
hidden class.
>
>
> JVM_RegisterLambdaProxyClassForArchiving - we can add an assert to
> check if lambdaProxyClass is a hidden class.
Added.
> JVM_IsCDSDumpingEnabled and JVM_IsCDSSharingEnabled - an alternative
> library/VM interface is to use internal system properties that
> jdk.internal.misc.VM::saveProperties reads and removes these flags.
I'd prefer to leave it as is as there's no perf issue if CDS is not
enabled. (More perf data below.)
> InnerClassLambdaMetafactory 247 /** Use the LambdaProxyClassArchive
> for including the lambda proxy class 248 * in CDS archive and
> retrieving the class from the archive. If the class 249 * is not found
> in the archive, it calls generatesInnerClass to create 250 * the
> class. 251 */ What about simpler comment: /* * Spins the lambda proxy
> class. * * This first checks if a lambda proxy class can be loaded
> from CDS archive. * Otherwise, generate the lambda proxy class. */
I've updated the comment.
> This adds a VM call for every lambda proxy class creation. Do you have
> any the performance measurement when CDS is not on? Any performance
> regression?
Here's the perf data with CDS disabled. The data is for loading 200
lambda proxy classes.
Results of " perf stat -r 50 bin/java -Xshare:off -cp TestD.jar TestD 200 "
1: 507523520 506743227 ( -780293) -- 156.992 156.453
( -0.539) --
2: 506280039 507494909 ( 1214870) +++ 155.799 156.510
( 0.711) ++
3: 506600572 507544573 ( 944001) ++ 155.100 156.878
( 1.778) +++++
4: 506240025 508590609 ( 2350584) +++++ 155.191 155.745
( 0.554) ++
5: 507113653 506861905 ( -251748) - 155.089 156.126
( 1.037) +++
6: 506331554 507727117 ( 1395563) +++ 155.588 155.452
( -0.136)
7: 506309111 506724285 ( 415174) + 156.407 156.914
( 0.507) +
8: 507805169 506926943 ( -878226) -- 155.796 155.922
( 0.126)
9: 506970117 507097707 ( 127590) 155.538 156.244
( 0.706) ++
10: 506951104 506754063 ( -197041) 155.429 156.747
( 1.318) ++++
============================================================
506812213 507246215 ( 434002) + 155.692 156.298
( 0.607) ++
instr delta = 434002 0.0856%
time delta = 0.607 ms 0.3896%
Updated webrevs:
delta: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev_delta.01/
full: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev.01/
thanks,
Calvin
More information about the core-libs-dev
mailing list