RFR JDK-8232222: Set state to 'linked' when an archived class is restored at runtime

Jiangli Zhou jianglizhou at google.com
Thu May 28 01:17:40 UTC 2020


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.

>
> 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