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