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

[Thomas Stüfe]

Thanks Ioi.

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
>>
>>
>