RFR(T): 8219554: Redundant lookup_common in SymbolTable::add

[Mikael Vidstedt]

> On Feb 21, 2019, at 4:55 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> Hi Claes,
> 
> Before your change, there's no API requirement that you must have called SymbolTable::lookup_common before calling SymbolTable::add, even though that is the behavior of the classFileParser.
> 
> So, in order to codify this new requirement, I think you should you should add an assert:
> 
> 
> +   assert(lookup_shared(name, len, hash) == NULL, "must have checked already");
>     Symbol* sym = SymbolTable::the_table()->do_add_if_needed(name, len, hash, c_heap, CHECK);

I’m sure this is a stupid(?) question, but I’m curious: What ensures the symbol isn’t removed/unloaded from the table between the earlier lookup and the SymbolTable::add call?

Cheers,
Mikael

> 
> Thanks
> - Ioi
> 
> 
> On 2/21/19 2:06 PM, Claes Redestad wrote:
>> Hi,
>> 
>> the arguments passed to SymbolTable::add has already been through a call
>> to lookup_common, so a small optimization is to remove the redundant
>> call inside the add method:
>> 
>> - the lookup_common already done in the class file parser ensures none
>> of the symbols lives in the shared archive.
>> - do_add_if_needed will do a quick get into the dynamic hash table anyhow.
>> 
>> Bug:   https://bugs.openjdk.java.net/browse/JDK-8219554
>> Patch: [1]
>> 
>> Testing: tier1+2
>> 
>> Thanks!
>> 
>> /Claes
>> 
>> [1]
>> hg diff -r cc9359f8c563 src/hotspot/share/classfile/symbolTable.cpp
>> --- a/src/hotspot/share/classfile/symbolTable.cpp    Thu Feb 21 14:16:44 2019 +0100
>> +++ b/src/hotspot/share/classfile/symbolTable.cpp    Thu Feb 21 23:12:29 2019 +0100
>> @@ -444,10 +444,7 @@
>>      const char *name = names[i];
>>      int len = lengths[i];
>>      unsigned int hash = hashValues[i];
>> -    Symbol* sym = SymbolTable::the_table()->lookup_common(name, len, hash);
>> -    if (sym == NULL) {
>> -      sym = SymbolTable::the_table()->do_add_if_needed(name, len, hash, c_heap, CHECK);
>> -    }
>> +    Symbol* sym = SymbolTable::the_table()->do_add_if_needed(name, len, hash, c_heap, CHECK);
>>      assert(sym->refcount() != 0, "lookup should have incremented the count");
>>      cp->symbol_at_put(cp_indices[i], sym);
>>    }
>