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

Thomas Stüfe thomas.stuefe at gmail.com
Tue Apr 2 05:47:04 UTC 2019


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

Thanks, Thomas


On Wed, Mar 27, 2019 at 10:01 PM Thomas Stüfe <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
>
>
>


More information about the hotspot-runtime-dev mailing list