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

[Nils Eliasson]

On 2020-05-06 22:20, Man Cao wrote:
> Hi,
>
> [@Laurent]
>> 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.
> You are welcome.
> I have run DaCapo at JDK tip, with default JVM options. I didn't see any
> noticeable difference in performance with and without my bugfix.
> This is probably due to significantly reduced code cache flushes with a
> large default ReservedCodeCacheSize (240MB) for +TieredCompilation.
> I checked the logs for tradesoap for runs without my bugfix, to count the
> number of completed flushes (NMethodSweeper::sweep_code_cache()):
> ~550 for runs with -XX:-TieredCompilation -XX:ReservedCodeCacheSize=40m on
> JDK11
> ~35 for runs with default options on JDK tip
> (+TieredCompilation, ReservedCodeCacheSize=240m)
> The flushes are reduced by more than 15X with the default options.
>
> [@Nils]
>> Looking at sweeper.cpp I see something that looks wrong. The _last_sweep
> zzcounter is updated even if no sweep was done. In low code cache usage
> scenarios that means will might never reach the threshold.
>> Can you try it out and see if things improve?
> The change makes sense to me. I can try it out together after resolving the
> next issue.
>
>> The sweeper should wake up
>> regularly, but now it is only awakened when hitting the SweepAggressive
>> threshold. This is wrong.
>> I suggest holding of the fix until all the problems have been ironed out.
> Could you elaborate what is the expected frequency to wake up the sweeper?
> Should we increase the default value for StartAggressiveSweepingAt instead?
The expected behaviour is that sweeper should be invoked depending on 
how many sweeper-stack scan has been done - that is tracked by the 
_time_counter field. In NMethodSweeper::possibly_sweep there is a 
heuristics that triggers a sweep more often if the code cache is getting 
full.

The expected behaviour for StartAggressiveSweepingAt is that we possibly 
start a sweep with a stack scan on every codeblob/nmethod allocation 
when the code cache is getting full. That is an attempt to quickly free 
up additional space in the code cache.

The first bug I found is that the _last_sweep counter should only be set 
when a sweep has been performed, otherwise the threshold might never be 
reached.

The second bug is that the sweeper thread sleeps. The only way to wake 
it up is through code paths that can only be reached when it is awake, 
or when StartAggressiveSweepingAt has been reached. This bug has gone 
unnoticed because the bug you found made SweepAggressive trigger when 
the codecache is 10% full, which most often happen fairly quickly.

To fix this the sweeper thread needs to be awakened whenever 
_time_counter has been updated.

However - that can only be a temporary fix - the transition to using 
mostly handshakes for synchronization can make the safepoints very rare. 
And then the heuristics is broken again.

Best regards,
Nils Eliasson

>
> As you suggested before, currently the sweeper is awakened for every new
> allocation in the code cache, after the usage is above 10%.
> This is definitely too frequent that it hurts performance, especially for
> the -XX:-TieredCompilation case.
> We do find that turning off code cache flushing in JDK11
> (-XX:-UseCodeCacheFlushing)
> could significantly improve performance, by ~20% for an important
> production workload configured with -XX:-TieredCompilation!
> Thus, we strongly support keeping the default flushing frequency low.
>
> -Man