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

[Ioi Lam]

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