RFR(S): 8164230: Convert TestCodeCacheRemSet_test to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Mon Aug 22 18:40:21 UTC 2016


Mikael,

Thank you for reviewing the fix!

Here are a new WebRev: 
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164230/webrev.01/

Changes:
1. g1CodeCacheRemSet.internal.hpp renamed to g1CodeRootSetTable.hpp and 
the class was renamed to G1CodeRootSetTable, #include  added and I was 
able to build images and gtests with --disable-precompiled-headers.

2. Moved G1CodeRootSet to the test method
3. Renamed getters in G1CodeRootSetTest to snake_case.

Could you please let me know your opinion?

Thank you.

Regards, Kirill

On 22.08.2016 16:43, Mikael Gerdin wrote:
> Hi Kirill,
>
> On 2016-08-22 14:57, Kirill Zhaldybin wrote:
>> Dear all,
>>
>> Could you please review this fix for 8164230?
>>
>> Since we need CodeRootSetTable class to be declared in the test it was
>> moved to new file src/share/vm/gc/g1/g1CodeCacheRemSet.internal.hpp.
>> This file is included by both test_g1CodeCacheRemSet.cpp and
>> g1CodeCacheRemSet.cpp.
>
> Please name the new file g1CodeRootSetTable.hpp and rename the class 
> G1CodeRootSetTable instead of inventing a new ".internal.hpp" format.
> Also, the new hpp file does not properly #include its dependencies and 
> therefore the build fails if precompiled headers are disabled, please 
> add the appropriate includes and forward declarations in order to make 
> it complete.
>
>
> The class G1CodeRootSetTest has a bunch of getters and a G1CodeRootSet 
> member, why are they like that instead of allocated on the stack like 
> the original version of the test?
>
>
> /Mikael
>
>>
>> G1CodeRootSetTest is a friend class for G1CodeRootSet so we can get
>> access to its internals.
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8164230
>> WebRev: 
>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164230/webrev.00/
>>
>> Thank you.
>>
>> Regards, Kirill




More information about the hotspot-gc-dev mailing list