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

[Thomas Stüfe]

Hi David,

On Mon, May 18, 2020 at 4:59 AM David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> FYI my previous email was written Friday/Saturday but got stuck until
> this morning. :(
>
>
Mailing lists behave strange lately.


> On 16/05/2020 6:33 pm, 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'd personally prefer put_default_if_absent() rather than relying on
> overloading. But not a blocker.
>

I agree, but this was Coleen's preference, and I see her point too.


>
> I'm unclear why the ClassLoaderStats APIs were changed from using
> pointers to objects/references ?
>

It was changed from pointers to objects we new and free ourself to just
in-table structures.

It just makes sense. For one, its easier on the eye since we do not have to
take care of manually managing space for the ClassLoaderStats objects.
Then, it is ever so slightly more efficient, since we use less memory (one
pointer size per Node plus whatever overhead the C-Heap allocation costs)
and have less indirections.


> src/hotspot/share/utilities/resourceHash.hpp
>
> +   // *p_created is new if entry was created, false if entry pre-existed.
>
> s/new/true/  - two occurrences.
>
>
Thanks, I'll fix that.

Cheers, Thomas