RFR (L) JDK-8195100: Use a low latency hashtable for SymbolTable

[Robbin Ehn]

Hi, another 2c,

On 08/07/2018 07:29 AM, Kim Barrett wrote:
>> On Aug 7, 2018, at 12:26 AM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Just adding my 2c in one place as I'm not looking at this in detail ...
>>
>> On 7/08/2018 12:18 PM, Kim Barrett wrote:
>>>> On Aug 6, 2018, at 12:39 PM, Gerard Ziemski <gerard.ziemski at oracle.com> wrote:
>>>>> src/hotspot/share/utilities/globalCounter.cpp
>>>>> 64   // Handle bootstrap
>>>>> 65   if (Threads::number_of_threads() == 0) {
>>>>> 66     return;
>>>>> 67   }
>>>>>
>>>>> What is this about?  number_of_threads only refers to Java threads,
>>>>> but the iteration should deal with there being none, right?  Or does
>>>>> this get called so early (because of SymbolTable changes) that even
>>>>> that iteration setup doesn't work?
>>>>>
>>>>> If not that, then maybe we're calling before the VMThread exists, and
>>>>> that's the real problem?  In which case a different test, differently
>>>>> located, seems necessary.

We use "Threads::number_of_threads() == 0" as bootstrap check in the VM, with 
stuff like:
       // cache can grow so we have to be more careful
       if (Threads::number_of_threads() == 0 ||
           SafepointSynchronize::is_at_safepoint()) {
         // we're single threaded or at a safepoint - no locking needed

I do not like that, but it's what we got. It is actually the only valid use of 
number_of_threads, except as an historical number.

Personally I would really like number_of_threads go away.

>>>>
>>>> We needed it when I was bringing up the code and I tried removing it to see whether we still need it and it seems that we do (details on the crash in the bug itself). Maybe Robbin can shed more light here?
>>> The problem here is that we're using the StringTable (and therefore
>>> GlobalCounter) before the main thread has been registered.  That
>>> registration happens in create_vm() (Threads::add(main_thread), line
>>> 3729), which follows the call to init_globals() (where the symbol
>>> table usage is occurring) in create_vm().
>>> Maybe moving the Threads::add earlier would fix the problem?  But this
>>> initialization code is very ordering sensitive, so I don't know if
>>> that would work.
>>
>> I'd suggest not attempting that. It might appear to work only for the bugs to become apparent much later.

I tried doing a proper fix by splitting up init methods in to parts and do it in 
a correct sequence, problem was, as you say, that I can't easily test it to be 
true. With small amount of time I deemed it not possible to ship a proper fix.

>>
>> I think this early VM usage needs to be pushed down to the SMR code to handle correctly, rather than hacking an initialization check into the GlobalCounter code.

Sorry I do not understand, please elaborate what you have in mind? (second part, 
yes it's a hack)

Thanks, Robbin

>>
>> David
>> -----
> 
> I agree with David on both counts.
>