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

[Erik Österlund]

Hi Vladimir,

Thanks for the review.

/Erik

> On 15 Jun 2020, at 19:44, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Good.
> 
> Thanks,
> Vladimir
> 
>> On 6/15/20 12:36 AM, Erik Österlund wrote:
>> Hi Vladimir,
>> Thank you for reviewing.
>> Here is a new webrev with your suggested commend changes and cleanup:
>> http://cr.openjdk.java.net/~eosterlund/8247319/webrev.01/
>> Incremental:
>> http://cr.openjdk.java.net/~eosterlund/8247319/webrev.00.01/
>> Thanks,
>> /Erik
>>> On 2020-06-12 21:35, Vladimir Kozlov wrote:
>>> 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