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

[Oli Gillespie]

On Mon, 13 Nov 2023 01:12:29 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 53:
> 
>> 51: 
>> 52: public:
>> 53:   static constexpr uint CLEANUP_DELAY_MAX_ENTRIES = 128;
> 
> This doesn't need to be public.

It's used in the test, do you prefer another approach like friend class?

> src/hotspot/share/oops/symbolHandle.hpp line 58:
> 
>> 56: 
>> 57:   // Conversion from a Symbol* to a SymbolHandleBase.
>> 58:   // Does not increment the current reference count if temporary.
> 
> This comment is no longer true for temp symbols, since adding to the delay queue increments the refcount.

Well spotted, thanks.

> src/hotspot/share/oops/symbolHandle.hpp line 115:
> 
>> 113:   }
>> 114: 
>> 115:   static void drain_cleanup_delay_queue() {
> 
> It's not obvious that draining the queue is useful. Unless there's a reason I'm missing, I suggest not doing so.

I don't feel too strongly either way but someone else previously suggested draining during the periodic task so I added it.
The benefit is not leaving Symbols hanging around in the queue indefinitely (though granted, a fixed number of them, so the memory waste is limited). The downside is a small piece of added code and work on the periodic task.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16398#discussion_r1406536331
PR Review Comment: https://git.openjdk.org/jdk/pull/16398#discussion_r1406536288
PR Review Comment: https://git.openjdk.org/jdk/pull/16398#discussion_r1406536415