[16] RFR: 8247319: Remove on-stack nmethod hotness counter sampling from safepoints

[Vladimir Kozlov]

I agree with changes.

Please also update comments [1] and clean up leftover [2] from 8244658 [3].

Thanks,
Vladimir

[1] http://hg.openjdk.java.net/jdk/jdk/file/09c54ee458aa/src/hotspot/share/runtime/sweeper.hpp#l38
     http://hg.openjdk.java.net/jdk/jdk/file/09c54ee458aa/src/hotspot/share/code/nmethod.hpp#l260

[2] http://hg.openjdk.java.net/jdk/jdk/file/09c54ee458aa/src/hotspot/share/runtime/vmOperations.hpp#l113
[3] https://bugs.openjdk.java.net/browse/JDK-8244658

On 6/12/20 12:29 AM, Erik Österlund wrote:
> Hi,
> 
> The sweeper is moving away from using safepoints for its heuristics. It used to count safepoints to figure out when to 
> sweep, but no longer does that. At the same time, we have for a while been removing more and more safepoints. Safepoints 
> are becoming increasingly rare events, dominated by when we need to GC (GuaranteedSafepointInterval is going to 
> disappear). The frequency of how often we need to GC does not have an obvious connection to how often we need to sweep 
> the code cache... any more.
> 
> What still remains from the safepoint-based heuristics is the nmethod hotness counter sampling that is performed in 
> safepoint cleanup. I would like to get rid of this.
> The rationale is that the use of hotness counters is kicking in when the code cache is starting to fill up quite a bit, 
> and there is a need to kill off nmethods heuristically, rather than because they are invalid. But when the code cache 
> fills up, we sweep more and more aggressively. And during these sweeper cycles, we perform nmethod marking using 
> handshakes. That operation also fills in hotness counters for all sampled nmethods.
> 
> In other words, when there is need for acting on the hotness counters, we are in a state where we may be getting more 
> nmethod hotness counter sample information from the sweeping cycles than we are from safepoint sampling. Conversely, 
> when code cache pressure is high and we need more samples, we might end up getting very few from safepoint based 
> sampling (because the heap is large). The correlation between safepoint frequency and code cache pressure is simply not 
> there any more. And for us to walk all stacks in the system in every single safepoint (which for ZGC is starting to 
> dominate pauses when I remove our stack sampling from safepoints), there better be a really good reason to do this 
> sampling in safepoints. And there simply isn't. So I propose we delete it, in favour of using the hotness counter 
> samples we get from the sweeping cycles instead, that are indeed proportional in frequency, to the code cache pressure.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8247319
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8247319/webrev.00/
> 
> Thanks,
> /Erik