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

[Vladimir Kozlov]

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
>