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

[Igor Ignatyev]

On Mon, 1 Mar 2021 19:58:19 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
>
> Mikhailo Seledtsov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed memory leak

Changes requested by iignatyev (Reviewer).

test/hotspot/gtest/concurrent_test_runner.inline.hpp line 89:

> 87: 
> 88: private:
> 89:   const TestRunnable* unitTestRunnable;

you made it a pointer to const TestRunnable, not a const pointer to TestRunnable.
Suggestion:

  TestRunnable* const unitTestRunnable;

test/hotspot/gtest/memory/test_virtualspace.cpp line 681:

> 679:   ConcurrentTestRunner testRunner(runnable, 30, 15000);
> 680:   testRunner.run();
> 681:   delete runnable;

wouldn't it be easier to allocate TestRunnable on stack and pass a pointer?
Suggestion:

  VirtualSpaceRunnable runnable();
  ConcurrentTestRunner testRunner(&runnable, 30, 15000);
  testRunner.run();

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

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