RFR: 8178349: Cache builtin class loader constraints to avoid re-initializing itable/vtable for shared classes
Ioi Lam
ioi.lam at oracle.com
Thu Apr 9 23:29:24 UTC 2020
Hi Yumin,
This looks good!
I have some suggestions so we can describe the design inside
the code and make the optimization easier to understand.
(Also, I think we need more test cases. I'll work with you separately on
that)
(1) Before your change, the JVM checks loader constraints in two places:
(a) during linking of a class
(b) when resolving constant pool entries.
Since your change involves only (a), I think it's better to rename
the function SystemDictionaryShared::record_loader_constraint ->
record_linking_constraint, and add these comments:
// Record class loader constraints that are checked inside
// InstanceKlass::link_class(), so that these can be checked quickly
// at runtime without laying out the vtable/itables.
void SystemDictionaryShared::record_linking_constraint(...)
(2) SystemDictionary::check_signature_loaders: parameter "this_klass" can be
changed to "class_being_linked". When calling this function from
linkResolver.cpp, we can say
/*class_being_linked=*/ NULL, // we are not linking a class
(3) We can restructure the checks in SystemDictionary::add_loader_constraint
to make them easier to understand:
FROM:
2289 // record constraint for app/platform-loader loaded class only.
2290 // dynamic dumping will re-layout of the vtable of the *copy*
of a class
2291 // in a vm_operation, the loader constraints alread recorded.
2292 if (Arguments::is_dumping_archive() && !THREAD->is_VM_thread()) {
2293 if (this_klass != NULL && !this_klass->is_shared() &&
2294 (is_system_class_loader(this_klass->class_loader()) ||
2295 is_platform_class_loader(this_klass->class_loader()))) {
2296 SystemDictionaryShared::record_loader_constraint(constraint_name,
2297 InstanceKlass::cast(this_klass),
2298 class_loader1, class_loader2);
2299 }
2300 }
TO:
if (Arguments::is_dumping_archive() && class_being_linked != NULL &&
!this_klass->is_shared()) {
SystemDictionaryShared::record_linking_constraint(constraint_name, ...);
}
All the conditions that are specific to the linking constraints
optimization should be moved into the CDS code.
In the beginning of record_linking_constraint():
void SystemDictionaryShared::record_linking_constraint(
// A linking constraint check is executed when:
// - klass extends or implements type S
// - klass overrides method S.M(...)
// - loader1 = klass->class_loader()
// - loader2 = S->class_loader()
// - loader1 != loader2
// - M's paramater(s) include an object type T
// We require that
// - whenever loader1 and loader2 try to
// resolve the type T, they must always resolve to
// the same InstanceKlass.
// NOTE: type T may or may not be currently resolved in
// either of these two loaders. The check itself does not
// try to resolve T.
assert(klass->class_loader() != NULL,
"should not be called for boot loader");
assert(klass->class_loader() == loader1(), "must be");
assert(loader1() != loader2(), "must be");
if (!is_system_class_loader(klass->class_loader() &&
!is_platform_class_loader(klass->class_loader()) {
// If klass is loaded by system/platform loaders, we can
// guarantee that klass and S must be loaded by the same
// respective loader between dump time and run time, and
// the exact same check on (name, loader0, loader1) will
// be executed. Hence, we can cache this check and execute
// it at runtime without walking the vtable/itables.
//
// This cannot be guaranteed for classes loaded by other
// loaders, so we bail.
return;
}
if (THREAD->is_VM_thread()) {
assert(DynamicDumpSharedSpaces, "must be");
// We are re-laying out the vtable/itables of the *copy* of
// a class during the final stage of dynamic dumping. The
// linking constraints for this class has already been recorded.
return;
}
(4) Similarly, for the runtime check, we can move the loader type check into
the CDS code:
InstanceKlass::link_class_impl() {
...
bool need_init_table = true;
if (is_shared() &&
SystemDictionaryShared::check_linking_constraints(this, THREAD)) {
need_init_table = false;
}
if (need_init_table) {
vtable().initialize_vtable(true, CHECK_false);
itable().initialize_itable(true, CHECK_false);
}
...
}
// Returns true IFF the linking constraints have been checked and no
// violations have been found.
bool
SystemDictionaryShared::check_linking_constraints(InstanceKlass* klass,
TRAPS) {
assert(klass->is_shared(), "must be");
if (... is boot class ..) {
// No class loader constraints checks are performed for boot
classes.
return true;
}
if (not (... is platform or system ...)) {
// linking constraints are not recorded for these classes.
return false;
}
....
if (!SystemDictionary::add_loader_constraint(....)) {
// Loader constraint violation has been found. The caller
// will re-layout the vtable/itables to produce the correct
// exception.
return false;
}
....
}
I changed the meaning of check_linking_constraints() so it's
more natural:
true means "good - no need to check"
false means "bad - need to check more")
(5) Also, for DTLoaderConstraint::operator==, we usually avoid operator
overloading in the HotSpot code, so I think it's better to rename it to
DTLoaderConstraint::equals().
Thanks
- Ioi
On 4/9/20 10:06 AM, Yumin Qi wrote:
> Hi, please review (please ignore my last email, the Subject is not
> correctly titled).
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8178349
> webrev: http://cr.openjdk.java.net/~minqi/8178349/webrev-01/
>
> Summary: When CDS link class, need re- intialize_vtable and
> intialize_itable. For a shared class the run for initialize table(s)
> in fact just check signature loaders of virtual or interface functions
> but we need go through many steps to reach the functions, iterate over
> the functions for signatures, resolve the symbol in loaders. Those
> steps also involved lock for SystemDictionary many times for resolving
> classes. (note for boot loaded shared classed no need to rerun).
> This patch record the loader constraints during dump time and in
> runtime, directly check loader constraints for the shared class
> (loaded by app or platform loader). The performance data showed:
>
> Results of " perf stat -r 40 bin/javac -J-Xshare:on
> -J-XX:SharedArchiveFile=javac2.jsa Bench_HelloWorld.java "
> 1: 2800027008 2793677554 ( -6349454) -- 390.665
> 390.210 ( -0.455)
> 2: 2799894892 2801783826 ( 1888934) 392.640
> 389.890 ( -2.750) ---
> 3: 2808688341 2792077758 (-16610583) ---- 392.430
> 390.210 ( -2.220) --
> 4: 2806708208 2790107502 (-16600706) ---- 395.210
> 390.060 ( -5.150) -----
> 5: 2807039108 2791228262 (-15810846) ---- 392.470
> 388.870 ( -3.600) ---
> 6: 2803446643 2783022861 (-20423782) ----- 392.550
> 388.070 ( -4.480) ----
> 7: 2809043394 2796696131 (-12347263) --- 394.810
> 389.960 ( -4.850) -----
> 8: 2798698381 2788160443 (-10537938) --- 393.430
> 390.130 ( -3.300) ---
> 9: 2797579793 2789786208 ( -7793585) -- 392.330
> 390.204 ( -2.126) --
> 10: 2808294825 2800296275 ( -7998550) -- 394.180
> 390.230 ( -3.950) ----
> ============================================================
> 2803938765 2792678505 (-11260259) --- 393.069
> 389.783 ( -3.287) ---
> instr delta = -11260259 -0.4016%
> time delta = -3.287 ms -0.8361%
>
> Results of " perf stat -r 40 bin/java -Xshare:on
> -XX:SharedArchiveFile=zprint2.jsa -cp ./zprint-filter-fixed.jar
> ZPrintBench "
> 1: 6548556888 6376039292 (-172517596) ---- 731.250 708.030
> (-23.220) -----
> 2: 6520993435 6391732011 (-129261424) --- 729.150 711.350
> (-17.800) ----
> 3: 6505744758 6355633193 (-150111565) ---- 729.440 707.090
> (-22.350) -----
> 4: 6545887467 6362941735 (-182945732) ----- 731.250
> 706.680 (-24.570) -----
> 5: 6497462252 6363834813 (-133627439) --- 729.940 709.540
> (-20.400) ----
> 6: 6567639848 6369214232 (-198425616) ----- 733.770
> 709.550 (-24.220) -----
> 7: 6511495888 6369700928 (-141794960) ---- 729.380 710.430
> (-18.950) ----
> 8: 6531525105 6342720734 (-188804371) ----- 730.070
> 706.090 (-23.980) -----
> 9: 6508499676 6322714877 (-185784799) ----- 728.700
> 705.560 (-23.140) -----
> 10: 6532457875 6430536065 (-101921810) --- 731.590 718.240
> (-13.350) ---
> ============================================================
> 6526992135 6368448595 (-158543539) ---- 730.453 709.247
> (-21.205) ----
> instr delta = -158543539 -2.4290%
> time delta = -21.205 ms -2.9030%
>
> Tests: hs-tier1,2,3,4
>
> Thanks
> Yumin
More information about the hotspot-runtime-dev
mailing list