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

[coleen.phillimore at oracle.com]

On 4/2/19 1:47 AM, Thomas Stüfe wrote:
> Hi Coleen, Andrew,
>
> thank you for reviewing my little change. Unfortunately, I had an 
> error in the space list verification method which needed fixing, so 
> here is a second version:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8221539--%5bmetaspace%5d-improve-metaspaceobj--is_metaspace_obj()-and-friends/webrev.01/webrev/

>
> Differences:
> - As Coleen requested: in allocation.cpp I replaced the comparison 
> this==NULL with a static helper method

I think you have to change the callers to not pass this as null.  So you 
can't do metaspaceobj->is_metaspace_object() because you're calling with 
"this" potentially NULL.

So remove this function:

  bool MetaspaceObj::is_metaspace_object() const {
- return Metaspace::contains((void*)this);
+ return MetaspaceObj::is_metaspace_object(this);
  }


> - I had mistype "envelope" as "envolope" in 
> "expand_envelope_to_include_node()". Since that sounded funny I 
> changed it.
> - The real bug was in VirtualSpaceList::verify() where I checked that 
> the extension of the envelope is as large as the current nodes. But 
> that is wrong, since the envelope never is shrunk (by design) and 
> nodes at the border of the envelope may have been unmapped. So the 
> real test should be to test if no node is outside the envelope.

So this envelope is an interesting concept and name.  It seems okay.  I 
guess over time, it won't give you a very good answer. Maybe you'll have 
to fix the boundaries someday.

Looks good though.  Thank you for making this improvement for performance.

Coleen
>
> Thanks, Thomas
>
>
> On Wed, Mar 27, 2019 at 10:01 PM Thomas Stüfe <thomas.stuefe at gmail.com 
> <mailto:thomas.stuefe at gmail.com>> 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
>
>