RFR (round 3), JEP-318: Epsilon GC

[Erik Österlund]

Hi Aleksey,

Just a few additional comments that nobody seems to have pointed out yet...

Spotted a weird ,\n; ending of the GC enum in 
test/lib/sun/hotspot/gc/GC.java.

You have some #ifdef COMPILER2 without including utilities/macros.hpp, 
which is a bit dangerous.

In your epsilonBarrierSet.cpp file, you should forward declare 
BarrierSetC1 and BarrierSetC2 in case you build this without COMPILER1 
and/or COMPILER2, like the other GCs.
Also not sure why #include "gc/shared/collectorPolicy.hpp" is included 
in epsilonBarrierSet.hpp. Seems like it's not needed there.
The epsilon/epsilonThreadLocalData.hpp does not have any includes, yet 
it seems to have a bunch of dependencies.

Thanks,
/Erik


On 2018-05-30 12:54, Aleksey Shipilev wrote:
> On 05/30/2018 12:42 PM, Per Liden wrote:
>>>>> Please keep the #if INCLUDE_XXX, XXX_ONLY and NOT_XXX sections sorted alphabetically. I.e. CMS -
>>>>> Epsilon - G1 ...
>>> Yup:
>>>     http://hg.openjdk.java.net/jdk/sandbox/rev/547ded4c0de6
>> Looks good!
>>
>> Minor nit, the indentation in vmStructs_gc.hpp looks off by 2 a few places:
>>
>> +  EPSILONGC_ONLY(VM_STRUCTS_EPSILONGC(nonstatic_field,
>>                                                               \
>> +                                    volatile_nonstatic_field,
>>                                                               \
>> +                                    static_field))
>>                                                               \
>>
>>
>> +  EPSILONGC_ONLY(VM_TYPES_EPSILONGC(declare_type,                         \
>> +                                  declare_toplevel_type,                  \
>> +                                  declare_integer_type))                  \
>>
>>
>> +  EPSILONGC_ONLY(VM_INT_CONSTANTS_EPSILONGC(declare_constant,     \
>> +                                          declare_constant_with_value))     \
> Oh. Took me a while to see it. Fixed:
>    http://hg.openjdk.java.net/jdk/sandbox/rev/14ebf728ce3f
>
> -Aleksey
>