RFR: 8315559: Delay TempSymbol cleanup to avoid symbol table churn [v4]

Oli Gillespie ogillespie at openjdk.org
Wed Nov 1 12:21:06 UTC 2023


On Tue, 31 Oct 2023 18:12:27 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Oli Gillespie has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Adress comments
>>   
>>   Fix indentation
>>   Improve tests
>>   Improve comment
>>   Remove redundant null check
>>   Improve naming
>>   Pop when >, not >= max len
>
> src/hotspot/share/oops/symbolHandle.hpp line 121:
> 
>> 119: 
>> 120:     // If the queue is now full, implement a one-in, one-out policy.
>> 121:     if (Atomic::add(&_cleanup_delay_len, 1, memory_order_relaxed) > _cleanup_delay_max_entries) {
> 
> Why is incrementing relaxed?  Now I have to think hard about whether there
> might be any ordering problems resulting from that.

A colleague suggested that was the appropriate constraint ("there is no transitive memory effects that ride on the counter"), but I don't understand it enough to defend it myself. Atomic::inc uses memory_order_conservative. Happy to go with whatever is preferred.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16398#discussion_r1378722803


More information about the hotspot-dev mailing list