RFR: 8213269: convert test/hotspot/jtreg/runtime/memory/RunUnitTestsConcurrently to gtest

[Igor Ignatyev]

On Fri, 5 Feb 2021 20:35:23 GMT, Mikhailo Seledtsov <mseledtsov at openjdk.org> wrote:

> This is a preliminary review. I would like to get the initial feedback before I proceed with conversion of the remaining tests.
> 
> Here is what I did so far:
>   - created a UnitTestThread and a main test runner, based on gtests with similar needs
>   - moved the original code from HotSpot internals (so called hotspot internal tests: src/hotspot/share/memory/virtualspace.cpp)
>     to the newly created gtest while wrapping it into a TestReservedSpace class. I did not change the code of the test.
>   - removed invocations from whitebox.cpp
> 
> Testing:
>   - ran GTestWrapper on usual platforms - All PASS
>   - ensured that ReservedSpaceConcurrent is in the logs and passed
> 
> After gathering the feedback my plan is:
> Plan:
>   - move the remaining internal Memory/VirtualSpace tests into a gTest
>     - I am thinking about using separate files for each test
>   - create a common file for UnitTestThread and MultiThreadTestRunner to reuse the code

Changes requested by iignatyev (Reviewer).

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 34:

> 32: class TestRunnable {
> 33: public:
> 34:   virtual void runUnitTest() {

I assume you meant for this class to be abstract, if so `runUnitTest` should be a pure virtual function.

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 1:

> 1: /*

for c++, we don't use camelCase in filenames, but rather use small_snake_case

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 97:

> 95:   long testDurationMillis;
> 96:   int nrOfThreads;
> 97:   TestRunnable* unitTestRunnable;

these also can be made `const`

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 60:

> 58: private:
> 59:   long testDuration;
> 60:   TestRunnable* runnable;

all these can be made `const` and initialized in the initializer list.

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 49:

> 47:   }
> 48: 
> 49:   virtual ~UnitTestThread() {}

why do you need virtual d-ctor here?

test/hotspot/gtest/concurrentTestRunner.inline.hpp line 100:

> 98: };
> 99: 
> 100: #endif // include guard

we tend to use the expression used in the corresponding `#if` / `#ifdef` as a comment in `#endif`

-------------

PR: https://git.openjdk.java.net/jdk/pull/2436