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

[Ioi Lam]

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);

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);
>    }