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

[Thomas Stüfe]

Hi Ioi & David,

after thinking this through some more, maybe I should fix up some callers
in this patch too as Ioi suggested. That way we will see if more functions
are needed.

I'll repost an updated webrev.

Thanks, Thomas


On Tue, May 12, 2020 at 8:10 AM Ioi Lam <ioi.lam at oracle.com> wrote:

> OK, looks good to me.
>
> Thanks
> - Ioi
>
> On 5/11/20 9:53 PM, Thomas Stüfe wrote:
>
> Hi Ioi,
>
> Thank you. I planned adapt CDS next :) but if possible I'd like to get
> this patch in since it already ran through our CI in this form.
>
> The first caller changed will be
> https://bugs.openjdk.java.net/browse/JDK-8244777, see:
> http://cr.openjdk.java.net/~stuefe/webrevs/8244777--reduce-classloaderstats-vm-operation-runtime/webrev.00/webrev
> <http://cr.openjdk.java.net/~stuefe/webrevs/8244777--reduce-classloaderstats-vm-operation-runtime/webrev.00/webrev/src/hotspot/share/classfile/classLoaderStats.cpp.udiff.html> .
> , but since that may require more JFR-specific changes which have nothing
> to do with ResourceHashTable I do it in a separate change.
>
> Cheers, Thomas
>
> On Tue, May 12, 2020 at 6:35 AM Ioi Lam <ioi.lam at oracle.com> wrote:
>
>> Hi Thomas,
>>
>> This is something that I've been wanting.
>>
>> Would it be possible for you to change the VM code so we will have at
>> least one example of using it for real? How about here :-)
>>
>>    DumpTimeSharedClassInfo* find_or_allocate_info_for(InstanceKlass* k) {
>>      DumpTimeSharedClassInfo* p = get(k);
>>      if (p == NULL) {
>> assert(!SystemDictionaryShared::no_class_loading_should_happen(),
>>               "no new classes can be loaded while dumping archive");
>>        put(k, DumpTimeSharedClassInfo());
>>        p = get(k);
>>        assert(p != NULL, "sanity");
>>        p->_klass = k;
>>      }
>>      return p;
>>    }
>>
>>
>> Thanks
>> - Ioi
>>
>> On 5/11/20 9: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.
>> >
>> > 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
>>
>>
>