RFR 8214310: SymbolTable: Use get and insert

[Gerard Ziemski]

hi Claes,

Thank you for the review.

> On Dec 10, 2018, at 12:31 PM, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi Gerard,
> 
> +    if (value->refcount() != PERM_REFCOUNT) {
> +      if (value->refcount() != 0) {
> +        assert(value->refcount() == 1, "expected newly created symbol");
> +        value->decrement_refcount();
> +      }
> +      assert(value->refcount() == 0, "expected dead symbol");
> +    }
> 
> I think this could be simplified:
> 
> +    if (value->refcount() == 1) {
> +      value->decrement_refcount();
> +      assert(value->refcount() == 0, "expected dead symbol");
> +    }

I like it.


> --
> 
> In do_add_if_needed, could &rehash_warning be set to true by one call
> and then reset to false by a subsequent call? Would it be more robust to
> call update_needs_hash inside the loop and rearrange accordingly?

The get(), then insert() will return the same value for this hint, so it will not get lost. The chances that we need to loop are small, but if we do, the hint value will also be the same.

New webrev: http://cr.openjdk.java.net/~gziemski/8214310_rev2
Testing:    Mach5 hs_tier1 in progress...


> Thanks!
> 
> /Claes
> 
> On 2018-12-10 18:14, Gerard Ziemski wrote:
>> Hi all,
>> Please review this fix, which is similar to 8213791, but here we implement it for the SymbolTable, whereas before we did it for the StringTable.
>> I’d like to see this go in JDK12, but if we determine that there is any part of it that is too controversial, I’m OK with waiting for JDK13
>> References:
>> bug id: https://bugs.openjdk.java.net/browse/JDK-8214310
>> webrev: http://cr.openjdk.java.net/~gziemski/8214310_rev1
>> tests:  passes Mach5 hs_tier1,2,3,4,5
>> Cheers