RFR(s): 8221539: [metaspace] Improve MetaspaceObj::is_metaspace_obj() and friends

[coleen.phillimore at oracle.com]

On 4/1/19 10:44 AM, Thomas Stüfe wrote:
>
>
> On Mon, Apr 1, 2019 at 4:03 PM <coleen.phillimore at oracle.com 
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
>
>     http://cr.openjdk.java.net/~stuefe/webrevs/8221539--%5bmetaspace%5d-improve-metaspaceobj--is_metaspace_obj()-and-friends/webrev.00/webrev/src/hotspot/share/memory/allocation.cpp.udiff.html
>
>     Apparently comparing this == NULL or other values is undefined
>     behaviour.  Luckily, I think there's just one call that can be
>     made static.
>
>
> Hi Coleen,
>
> What would be the "right" way to do this? A static helper taking a 
> MetaspaceObj* and comparing it to NULL?

Yes.
>
> Note that we seem to compare this == NULL in a couple of places. If 
> this is UB those should be fixed too.

We had a pass at fixing some of these places.  I agree we should fix 
them all (not this patch of course).

thanks,
Coleen

>
> Thank you, Thomas
>
>
>
>     The rest looks good.
>     Coleen
>
>
>
>     On 3/27/19 5:01 PM, Thomas Stüfe wrote:
>     > Hi all,
>     >
>     > May I please have reviews for this small optimization:
>     >
>     > cr:
>     >
>     http://cr.openjdk.java.net/~stuefe/webrevs/8221539--%5bmetaspace%5d-improve-metaspaceobj--is_metaspace_obj()-and-friends/webrev.00/webrev/index.html
>     > Issue: https://bugs.openjdk.java.net/browse/JDK-8221539
>     >
>     > There are several functions which, given an unknown pointer
>     assumed to be a
>     > metaspace object, check if the pointer is indeed a metaspace
>     object by
>     > walking the VirtualSpaceList and checking ranges.
>     >
>     > This patch adds checks which weed out the obvious cases to avoid
>     needlessly
>     > walking the vs list.
>     >
>     > Patch also adds verifications for the VirtualSpaceList in debug
>     cases.
>     > Those run only when a new node has been added to the list, or
>     when a node
>     > has been purged, so very sparingly.
>     >
>     > When purging nodes, I removed a small unnecessary and
>     inefficient check
>     > which checked whether (one of the) purged nodes was still in the
>     list.
>     > Since we now as part of the new VirtualSpaceNode::verify() walk
>     this list,
>     > the check is unnecessary.
>     >
>     > Thanks, Thomas
>