RFR(XXS): 8244278: Excessive code cache flushes and sweeps

Wed May 6 09:34:07 UTC 2020

Thanks Man for your results.

I will try your fix on jdk15 repo and run my Marlin tests & benchmark to
see if there are any gain in these cases.

Cheers,
Laurent

Le mar. 5 mai 2020 à 21:00, Man Cao <manc at google.com> a écrit :

> Hi Laurent, Nils,
>
> > Using 40mb code cache looks quite small compared to default values (256mb
> ?).
> The 40MB code cache size is for -XX:-TieredCompilation, for the DaCapo
> runs.
> "java -XX:-TieredCompilation -XX:+PrintFlagsFinal |& grep
> ReservedCodeCacheSize" shows the default size is 48MB for
> -TieredCompilation.
> So 40MB is not particularly small compared to 48MB.
>
> The Blaze runs use "-XX:+TieredCompilation" and a Google-default code cache
> size of 480MB (we have doubled the default size).
> This actually tests a case that is closer to the OpenJDK default.
>
> > Could you run 1 quick experiment with default code cache settings to see
> if there is no regression in the main use case?
> Yes, I just launched an experiment running DaCapo at JDK source tip, only
> with "-Xms4g -Xmx4g" and some logging flags.
> This is a meaningful experiment as it uses the real OpenJDK default flags,
> and the most up-to-date source.
>
> > Why do you expect code cache usage would increase a lot? The sweeper
> > still wakes up regularly and cleans the code cache. The code path fixed
> > is just about sweeping extra aggressively under some circumstances. Some
> > nmethod might live a little longer, but they will still be cleaned.
> I found the aggressive sweeping deoptimized a non-trivial number of
> nmethods, which kept the usage low.
> After my bugfix, the sweeper only wakes up when the usage reaches
> (100-StartAggressiveSweepingAt)% of code cache capacity, which default to
> 90%.
> This means many nmethods will not be cleaned until there is pressure in
> code cache usage.
>
> > One number you could add to your benchmark numbers is the number of
> > nmethods reclaimed and code cache usage. I expect both to remain the
> same.
> The benchmark result htmls already show the code cache usage (Code Cache
> Used (MB)).
> You can CTRL-F for "Code Cache" in the browser.
>
> There is a significant increase in code cache usage:
> For DaCapo, up to 13.5MB (for tradesoap) increase for the 40MB
> ReservedCodeCacheSize.
> For Blaze, the increase is 33MB-80MB for the 480MB ReservedCodeCacheSize.
>
> The code cache usage metric is measure like this (we added an hsperfdata
> counter for it), at the end of a benchmark run:
>   size_t result = 0;
>   FOR_ALL_ALLOCABLE_HEAPS(heap) {
>     result += (*heap)->allocated_capacity();
>   }
>   _code_cache_used_size->set_value(result);
>
> I also looked at the logs, it shows that the bugfix eliminated almost all
> of the code cache flushes. They also contain the number of nmethods
> reclaimed.
> E.g., for tradesoap:
>
> Without the fix:
> Code cache sweeper statistics:
>
>   Total sweep time:                1902 ms
>   Total number of full sweeps:     23301
>   Total number of flushed methods: 7080 (thereof 7080 C2 methods)
>   Total size of flushed methods:   20468 kB
>
> With the fix:
>
> Code cache sweeper statistics:
>   Total sweep time:                0 ms
>   Total number of full sweeps:     0
>   Total number of flushed methods: 0 (thereof 0 C2 methods)
>   Total size of flushed methods:   0 kB
>
>
> Anyway, just to reiterate, we think the improvement in throughput and CPU
> usage is well worth the increase in code cache usage.
> If the increase causes any problem, we would advise users to accept the
> increase and fully provision memory for the entire ReservedCodeCacheSize.
>
> -Man
>
>
> On Tue, May 5, 2020 at 3:18 AM Nils Eliasson <nils.eliasson at oracle.com>
> wrote:
>
> > Hi Man,
> >
> > Why do you expect code cache usage would increase a lot? The sweeper
> > still wakes up regularly and cleans the code cache. The code path fixed
> > is just about sweeping extra aggressively under some circumstances. Some
> > nmethod might live a little longer, but they will still be cleaned.
> >
> > Without your bugfix the sweeper will be notified for every new
> > allocation in the codecache as soon as code cache usages has gone beyond
> > 10%. That could in the worst case be one sweep for every allocation.
> >
> > One number you could add to your benchmark numbers is the number of
> > nmethods reclaimed and code cache usage. I expect both to remain the
> same.
> >
> > Best regards,
> > Nils
> >
> >
> >
> > On 2020-05-04 21:21, Man Cao wrote:
> > > Hi,
> > >
> > > Thanks for the review!
> > > Yes, the code change is trivial, but runtime behavior change is
> > > considerable.
> > > In particular, throughput and CPU usage could improve, but code cache
> > usage
> > > could increase a lot. In our experience, the improvement in throughput
> > and
> > > CPU is well worth the code cache increase.
> > >
> > > I have attached some benchmarking results in JBS. They are based on
> > JDK11.
> > > We have not rolled out this fix to our production JDK11 yet, as I'd
> like
> > to
> > > confirm that this large change in runtime behavior is OK with the
> OpenJDK
> > > community.
> > > We are happy to share some performance numbers from production workload
> > > once we have them.
> > >
> > > -Man
> > >
> > >
> > > On Mon, May 4, 2020 at 4:11 AM Laurent Bourgès <
> > bourges.laurent at gmail.com>
> > > wrote:
> > >
> > >> Hi,
> > >>
> > >> Do you have performance results to justify your assumption "could
> > >> significantly improve performance" ?
> > >>
> > >> Please share numbers in the jbs bug
> > >>
> > >> Laurent
> > >>
> > >> Le sam. 2 mai 2020 à 07:35, Man Cao <manc at google.com> a écrit :
> > >>
> > >>> Hi all,
> > >>>
> > >>> Can I have reviews for this one-line change that fixes a bug and
> could
> > >>> significantly improve performance?
> > >>> Webrev: https://cr.openjdk.java.net/~manc/8244278/webrev.00/
> > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8244278
> > >>>
> > >>> It passes tier-1 tests locally, as well as
> > >>> "vm/mlvm/meth/stress/compiler/deoptimize" (for the original
> > JDK-8046809).
> > >>>
> > >>> -Man
> > >>>
> >
> >
>