RFR JDK-8232222: Set state to 'linked' when an archived class is restored at runtime
Ioi Lam
ioi.lam at oracle.com
Thu May 28 01:35:44 UTC 2020
On 5/27/20 6:17 PM, Jiangli Zhou wrote:
> On Wed, May 27, 2020 at 1:56 PM Ioi Lam <ioi.lam at oracle.com> wrote:
>> On 5/26/20 6:21 PM, Jiangli Zhou wrote:
>>
>>> Focusing on the link state for archived classes in this thread, I
>>> updated the webrev to only set archived boot classes to 'linked' state
>>> at restore time. More investigations can be done for archived classes
>>> for other builtin loaders.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8232222
>>> http://cr.openjdk.java.net/~jiangli/8232222/webrev.02/
>>>
>>> Please let me know if there is any additional concerns to the change.
>>>
>>> Best regards,
>>> Jiangli
>>>
>> Hi Jiangli,
>>
>> I think the change is fine. I am wondering if this
>>
>> 2530 if (!BytecodeVerificationLocal &&
>> 2531 loader_data->is_the_null_class_loader_data()) {
>> 2532 _init_state = linked;
>> 2533 }
>>
>>
>> can be changed to
>>
>> if (!BytecodeVerificationLocal &&
>> loader_data->is_the_null_class_loader_data() &&
>> !JvmtiExport::should_post_class_prepare())
>>
>> That way, there's no need to change systemDictionary.cpp.
>>
>>
> I was going to take the suggestion, but realized that it would add
> unnecessary complications for archived boot classes with class
> pre-initialization support. Some agents may set
> JvmtiExport::should_post_class_prepare(). It's worthwhile to support
> class pre-init uniformly for archived boot classes with
> JvmtiExport::should_post_class_prepare() enabled or disabled.
This would introduce behavioral changes when JVMTI is enabled:
+ The order of JvmtiExport::post_class_prepare is different than before
+ JvmtiExport::post_class_prepare may be called for a class that was not
called before (if the class is never linked during run time)
+ JvmtiExport::post_class_prepare was called inside the init_lock, now
it's called outside of the init_lock
Thanks
- Ioi
>
>> BTW, I was wondering where the performance came from, so I wrote an
>> investigative patch:
>>
>> diff -r 0702191777c9 src/hotspot/share/oops/instanceKlass.cpp
>> --- a/src/hotspot/share/oops/instanceKlass.cpp Thu May 21 15:56:27
>> 2020 -0700
>> +++ b/src/hotspot/share/oops/instanceKlass.cpp Wed May 27 10:48:57
>> 2020 -0700
>> @@ -866,6 +866,13 @@
>> return true;
>> }
>>
>> + if (UseSharedSpaces && !BytecodeVerificationLocal &&
>> is_shared_boot_class()) {
>> + Handle h_init_lock(THREAD, init_lock());
>> + ObjectLocker ol(h_init_lock, THREAD, h_init_lock() != NULL);
>> + set_init_state(linked);
>> + return true;
>> + }
>> +
>> // trace only the link time for this klass that includes
>> // the verification time
>> PerfClassTraceTime vmtimer(ClassLoader::perf_class_link_time(),
>>
>>
>> Benchmarking results (smaller numbers are better):
>>
>> (baseline vs your patch)
>>
>> baseline jiangli baseline jiangli
>> 1: 58514375 57755638 (-758737) ----- 40.266 40.135 (
>> -0.131) -
>> 2: 58506426 57754623 (-751803) ----- 40.367 39.417 (
>> -0.950) -----
>> 3: 58498554 57759735 (-738819) ----- 40.513 39.970 (
>> -0.543) ---
>> 4: 58491265 57751296 (-739969) ----- 40.439 40.268 (
>> -0.171) -
>> 5: 58500588 57750975 (-749613) ----- 40.569 40.080 (
>> -0.489) --
>> 6: 58497015 57744418 (-752597) ----- 41.097 40.147 (
>> -0.950) -----
>> 7: 58494335 57749909 (-744426) ----- 39.983 40.214
>> ( 0.231) +
>> 8: 58500401 57750305 (-750096) ----- 40.235 40.417
>> ( 0.182) +
>> 9: 58490728 57767463 (-723265) ----- 40.354 39.928 (
>> -0.426) --
>> 10: 58497858 57746557 (-751301) ----- 40.756 39.706 (
>> -1.050) -----
>> ============================================================
>> 58499154 57753091 (-746062) ----- 40.457 40.027 (
>> -0.430) --
>> instr delta = -746062 -1.2753%
>> time delta = -0.430 ms -1.0619%
>>
>>
>> (baseline vs my patch)
>>
>> baseline ioi baseline ioi
>> 1: 58503574 57821124 (-682450) ----- 40.554 39.783 (
>> -0.771) -----
>> 2: 58499325 57819459 (-679866) ----- 40.092 40.325
>> ( 0.233) ++
>> 3: 58492362 57811978 (-680384) ----- 40.546 39.826 (
>> -0.720) -----
>> 4: 58488655 57828878 (-659777) ----- 40.270 40.550
>> ( 0.280) ++
>> 5: 58501567 57830179 (-671388) ----- 40.382 40.145 (
>> -0.237) --
>> 6: 58496552 57808774 (-687778) ----- 40.702 40.527 (
>> -0.175) -
>> 7: 58482701 57808925 (-673776) ----- 40.268 39.849 (
>> -0.419) ---
>> 8: 58493831 57807810 (-686021) ----- 40.396 39.940 (
>> -0.456) ---
>> 9: 58489388 57811354 (-678034) ----- 40.575 40.078 (
>> -0.497) ---
>> 10: 58482512 57795489 (-687023) ----- 40.084 40.247
>> ( 0.163) +
>> ============================================================
>> 58493046 57814396 (-678650) ----- 40.386 40.126 (
>> -0.260) --
>> instr delta = -678650 -1.1602%
>> time delta = -0.260 ms -0.6445%
>>
>>
>> (your patch vs my patch)
>>
>> jiangli ioi jiangli ioi
>> 1: 57716711 57782622 ( 65911) ++++ 41.042 40.302 (
>> -0.740) -----
>> 2: 57709666 57780196 ( 70530) ++++ 40.334 40.965 (
>> 0.631) ++++
>> 3: 57716074 57803315 ( 87241) +++++ 40.239 39.823 (
>> -0.416) ---
>> 4: 57725152 57782719 ( 57567) +++ 40.430 39.805 (
>> -0.625) ----
>> 5: 57719799 57787187 ( 67388) ++++ 40.138 40.003 (
>> -0.135) -
>> 6: 57721922 57769193 ( 47271) +++ 40.324 40.207 (
>> -0.117) -
>> 7: 57716438 57785212 ( 68774) ++++ 39.978 40.149 (
>> 0.171) +
>> 8: 57713834 57778797 ( 64963) ++++ 40.359 40.210 (
>> -0.149) -
>> 9: 57711272 57786376 ( 75104) ++++ 40.575 40.724 (
>> 0.149) +
>> 10: 57711660 57780548 ( 68888) ++++ 40.291 40.091 (
>> -0.200) -
>> ============================================================
>> 57716252 57783615 ( 67363) ++++ 40.370 40.226 (
>> -0.144) -
>> instr delta = 67363 0.1167%
>> time delta = -0.144 ms -0.3560%
>>
>>
>> These numbers show that the majority of the time spent (678650
>> instructions) inside InstanceKlass::link_class_impl is spent from the
>> PerfClassTraceTime. Walking of the class hierarchy and taking the
>> h_init_lock only takes about 67363 instructions).
>>
>> Due to this finding, I filed two more RFEs:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8246019
>> PerfClassTraceTime slows down VM start-up
>>
> It's related to JDK-8246020, and I've commented on the bug (see
> JDK-8246020 comments). UsePerfData for perf data collection is common
> in cloud usages. It's better to keep UsePerfData enabled by default.
>
>> https://bugs.openjdk.java.net/browse/JDK-8246015
>> Method::link_method is called twice for CDS methods
>
> That was addressed as part of the initial change for JDK-8232222:
> http://cr.openjdk.java.net/~jiangli/8232222/weberv.02/src/hotspot/share/oops/instanceKlass.cpp.frames.html
>
> It's cleaner to handle it separately, so I removed it from the latest
> version. I've assigned JDK-8246015 to myself and will address it
> separately. Thanks for recording the separate bug.
>
> Thanks!
> Jiangli
>
>>
>> Thanks
>> - Ioi
More information about the hotspot-runtime-dev
mailing list