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

[Coleen Phillimore]

On Mon, 13 Nov 2023 01:12:11 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Oli Gillespie has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix indentation, rename test helper
>
> src/hotspot/share/oops/symbolHandle.hpp line 48:
> 
>> 46: class SymbolHandleBase : public StackObj {
>> 47:   static Symbol* volatile _cleanup_delay_queue[];
>> 48:   static volatile uint _cleanup_delay_index;
> 
> Putting the delay queue implementation in SymbolHandleBase<> makes unnecessary and unused
> data and possibly copies of the code. It is only used in the case where the template parameter is true.
> Better would be to put the cleanup delay queue in a separate, non-templated, class. The entire
> implementation of the queue could then be in the .cpp file. (I suggest the overhead of an out-of-line
> call to add to the queue is in the noise, given that adding to the queue performs 3 atomic RMW
> operations.)  So something like this:
> 
> 
> // In symbolHandle.hpp.
> class TempSymbolCleanupDelayer : AllStatic {
>   // Or make these file-scoped statics in the .cpp file.
>   static const uint QueueSize = 128;
>   static Symbol* volatile _queue[];
>   static volatile uint _index;
> 
> public:
>   static void delay_cleanup(Symbol* s);
> };
> 
> // In symbolHandle.cpp.
> Symbol* volatile TempSymbolCleanupDelayer::_queue[QueueSize] = {};
> volatile uint TempSymbolCleanupDelayer::_index = 0;
> 
> void TempSymbolCleanupDelayer::delay_cleanup(Symbol* sym) {
>   assert(sym != nullptr, "precondition");
>   sym->increment_refcount();
>   uint i = Atomic::add(&_index, 1u) % QueueSize;
>   Symbol* old = Atomic::xchg(&_queue, sym);
>   Symbol::maybe_decrement_refcount(old);
> }
> 
> 
> Code is completely untested.  It incorporates suggestions I'm making elsewhere in this review too.

This seems like a reasonable suggestion.  It would be better in as a singleton in the cpp file.

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

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