RFR(s): 8244733: Add ResourceHashtable::xxx_if_absent (was: ResourceHashtable::compute_if_absent)

[Robbin Ehn]

Hi Thomas,

You have two copies of put_if_absent, if you e.g. add a private one instead:

   V* priv_put_if_absent(K const& key, const V * value, bool* p_created) ...
       *ptr = value != NULL ? new (ALLOC_TYPE, MEM_TYPE) Node(hv, key, 
*value) : new (ALLOC_TYPE, MEM_TYPE) Node(hv, key);

You only need one.

/Robbin

On 2020-05-16 10:33, Thomas Stüfe wrote:
> Hi all.
> 
> new version:
> http://cr.openjdk.java.net/~stuefe/webrevs/8244733--add-resourcehashtable--compute_if_absent/webrev.02/webrev/
> 
> with Coleens feedback worked in:
> 
> I removed the compute_if_absent() variant since of the new three methods it
> seems the least useful one; this leaves us with a variant which
> default-creates the value if not found, and one with puts a caller provided
> value if not found. I also removed the default argument for the p_created
> output.
> 
> As per Coleen's request, I named the methods both "put_if_absent". See
> comments at the method; I hope the intent is still clear.
> 
> I also fixed up the callers and the test.
> 
> Thanks, Thomas
> 
> 
> 
> On Thu, May 14, 2020 at 10:08 AM Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
> 
>> Hi,
>>
>> This is a continuation of the review thread:
>> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-May/039444.html
>>
>> I changed the title of the bug somewhat hence the new thread.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244733
>> Webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8244733--add-resourcehashtable--compute_if_absent/webrev.01/webrev/
>>
>> This now adds three new functions to ResourceHashTable:
>>
>> new_if_absent(), compute_if_absent(), put_if_absent()
>>
>> which work very similar to their equivalents in j.u.Map.
>>
>> The advantage they offer is that for a typical "if map does not contain X,
>> add X+value" pattern we only do the lookup once.
>>
>> --
>>
>> So I extended the scope of the item. After playing around with various
>> call sites I realized that David's proposal of put_if_absent() would be
>> useful, as well as a variant which just adds a default constructed value.
>> All three are tested in gtest and used in real call sites.
>>
>> Ran tests tonight in our CI, all went through.
>>
>> Thanks, Thomas
>>
>>
>>