RFR(XS) : 8177711 : Convert TestVirtualSpaceNode_test to GTest

[Thomas Stüfe]

Hi Igor,

Thanks for taking this on! This has been long overdue.

That said, with this particular test, I feel that since 8198423 its
usefulness is greatly diminished, and there is a bit of a danger of
getting it wrong, messing up global structures in metaspace and
affecting follow up tests. Metaspace is not (anymore) designed to be
used on this isolated level.

I really thought long and hard about this. Unfortunately, this test
cannot be done in an isolated fashion anymore without affecting global
metaspace - sure you encountered that yourself, hence the terrible
(sorry :-) ChunkManagerRestorer workaround. And I do not see a better
way to do this.

All these tests do now is to ensure that a particular notion of chunk
geometry and allocation behavior is adhered to - stuff which may
change anyway since it is implementation detail. I do not see any
really worthwhile tests. And the fact that since 8198423 any
allocation from a VirtualSpaceNode can cause allocation of padding
chunks, which get added to the global freelist and hence change global
state, makes matters quite complicated.

If you want to get ahead with this change, here some remarks:

----

all_vsn_is_committed_half_is_used_by_chunks - can be completely removed IMHO.

----

MetachunkRemover seems to be nowhere used, can this be removed?

----

does this compile with non-pch? Test file seems to miss some headers,
e.g. virtualSpaceNode.hpp. Not sure if they come in implicitly.

----

But still, bottomline, I'd rather remove the test. When doing 8198423,
I added a new gtest to make up for the now-teethless VSN gtest, see
test_metaspace_allocation.cpp. There, I still stress Metaspace but a
layer higher, using the same code paths the rest of the code uses when
allocating metaspace.

Kind Regards, Thomas
On Wed, Oct 24, 2018 at 8:50 PM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>
> http://cr.openjdk.java.net/~iignatyev//8177711/webrev.00/index.html
> > 405 lines changed: 244 ins; 159 del; 2 mod;
>
>
> Hi all,
>
> could you please review this small and straightforward patch which converts TestVirtualSpaceNode_test into GTest? TestVirtualSpaceNode_test have been removed from executed by 8198423[1] and some of its asserts aren't true any more. I have them commented them out, and filed 8212932[2] to clean them up.
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8177711/webrev.00/index.html
> JBS: https://bugs.openjdk.java.net/browse/JDK-8177711
>
> Thanks,
> -- Igor
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8198423
> [2] https://bugs.openjdk.java.net/browse/JDK-8212932
>