RFR(xs): 8244733: Add ResourceHashtable::compute_if_absent

[David Holmes]

Sorry ignore this.

There are different tradeoffs with different patterns in relation to 
computing the value conditionally or unconditionally.

Consider this reviewed.

Thanks,
David
-----

On 12/05/2020 4:32 pm, David Holmes wrote:
> On 12/05/2020 3:15 pm, Thomas Stüfe wrote:
>> On Tue, May 12, 2020 at 7:00 AM David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>>     Hi Thomas,
>>
>>     On 12/05/2020 2:25 pm, Thomas Stüfe wrote:
>>      > Hi all,
>>      >
>>      > May I please have reviews for this enhancement:
>>      >
>>      > JBS: https://bugs.openjdk.java.net/browse/JDK-8244733
>>      > Webrev:
>>      >
>>     
>> http://cr.openjdk.java.net/~stuefe/webrevs/8244733--add-resourcehashtable--compute_if_absent/webrev.00/webrev/ 
>>
>>      >
>>      > A common pattern when using ResourceHashTable is:
>>      >
>>      > if (table.get(k) == NULL) table.add(k,v);
>>      >
>>      > This runs the lookup code twice. By providing a new method which
>>     combines
>>      > these two operations we would save one lookup.
>>      >
>>      > The new method is named ResourceHashTable::compute_if_absent()
>>     and behaves
>>      > very similar to the similarly named variant in j.u.Map.
>>
>>     The code to replace the pattern above would be putIfAbsent. The 
>> general
>>     motivation for computeIfAbsent is to avoid the execution of an
>>     expensive
>>     mapping function if the mapping already exists. Perhaps you need 
>> both?
>>
>>
>> The places I had in mind (ClassLoaderStats, CDS) all needed a 
>> computeIfAbsent. I have not seen any places which would benefit from a 
>> putIfAbsent, but I can look again.
> 
> Okay but the "common pattern" you used to motivate this matches 
> putIfAbsent not computeIfAbsent. That pattern would require the 
> introduction of a compute function object to use computeIfAbsent - as 
> per your 8244777 example code. Stylistically I find the need to 
> introduction a function object just to use this API somewhat grating.
> 
>> I would like to avoid adding functions proactively though. In my mind 
>> putIfAbsent is less useful than computeIfAbsent, only to be preferred 
>> if construction needs more information than a mapping function can 
>> provide.
> 
> I would flip that around and say that computeIfAbsent is only useful if 
> concurrent puts are possible and you want to avoid computing the new 
> value unnecessarily in each case. I'm not clear from the potential 
> call-sites whether concurrent puts are in issue here or not - but if not 
> then putIfAbsent is all you need.
> 
> David
> 
>> Thanks, Thomas
>>
>>     David
>>
>>      > Note: Actually replacing caller code will happen in a separate
>>     RFE. The
>>      > first caller to use this will be the ClassLoaderStats VM
>>     operation - I was
>>      > able to significantly reduce runtime for that vm op. But since
>>     that is JFR
>>      > territory and this is more of a runtime topic I wanted to keep
>>     the review
>>      > separate.
>>      >
>>      > Thanks, Thomas
>>      >
>>