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

[Mikhailo Seledtsov]

On Fri, 5 Feb 2021 21:03:56 GMT, Igor Ignatyev <iignatyev 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
>
> hi Misha,
> 
> I haven't finished (pre)reviewing it yet, however, I have found a few things that, I belive, should be changed.

I have updated the changes with recommendations from Igor. I also have:
  - converted all 3 tests, which also includes platforms-specific tests
  - using GTest EXPECT/ASSERT macros
  - removed remnants of the original tests
  - general cleanup
  - ran build-and-test on these tests, plus a number of additional builds
Please review when you have a chance: @iignatev @hseigel

> test/hotspot/gtest/runtime/test_reservedSpaceConcurrent.cpp line 46:
> 
>> 44:   static void release_memory_for_test(ReservedSpace rs) {
>> 45:     if (rs.special()) {
>> 46:       guarantee(os::release_memory_special(rs.base(), rs.size()), "Shouldn't fail");
> 
> you shouldn't use hotspot's `gurantee/assert`, and use gtest provided `ASSERT`/`EXPECT` instead.

Will do.

> test/hotspot/gtest/runtime/test_reservedSpaceConcurrent.cpp line 208:
> 
>> 206:   void run_unit_test() {
>> 207:     TestReservedSpace::test_reserved_space();
>> 208:   }
> 
> what's the point of having this extra member function? can't we just call `TestReservedSpace::test_reserved_space` directly from main_run?

I reworked the classes to be able to reuse the concurrency driver and threads code.

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

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