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

[Erik Österlund]

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