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

[Erik Österlund]

Hi Aleksey,

On 2018-05-30 14:16, Aleksey Shipilev wrote:
> Thanks!
>
> On 05/30/2018 01:41 PM, Erik Österlund wrote:
>> Spotted a weird ,\n; ending of the GC enum in test/lib/sun/hotspot/gc/GC.java.
> It is customary to have the "open" enum, so that additions do not have to change the previous line.
> But okay, let's do the closed form:
>    http://hg.openjdk.java.net/jdk/sandbox/rev/28d61698c79e
>
>> You have some #ifdef COMPILER2 without including utilities/macros.hpp, which is a bit dangerous.
> Right. CI does not complain building Epsilon without compiler2, so I guess we are saved by some
> other transitive dependency. Made it explicit:
>    http://hg.openjdk.java.net/jdk/sandbox/rev/293ba598ee68
>
>> 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.
> I don't see why? There is a forward declaration of BarrierSetC1/C2 in barrierSet.hpp that
> epsilonBarrierSet.cpp includes. G1, for example, forward-declares *G1*BarrierSetC1/C2, but Epsilon
> uses the top classes for BarrierSetAssembler/C1/C2.

You are right.

>
>> 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.
> Right. Added/removed:
>    http://hg.openjdk.java.net/jdk/sandbox/rev/50ad8ee62366

Thanks for fixing. Looks good.

/Erik

>
> Thanks,
> -Aleksey
>