RFR 8214310: SymbolTable: Use get and insert

[coleen.phillimore at oracle.com]

On 12/10/18 1:31 PM, Claes Redestad 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");
> +    }
>
> -- 
>
> 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 needs_rehashing is never turned back to false, so I think it's fine 
outside the loop.  To be more consistent with StringTable, maybe it 
should go into the loop though.

This change looks really good to me.  Should you remove get_insert_lazy 
with this change and internal_get_insert with this change?

Or file a different RFE.  There are probably tests for this API.

thanks,
Coleen
>
> 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
>>