Hi Kim,
>> On Nov 26, 2018, at 4:39 PM, Roman Kennke <rken...@redhat.com> wrote:
>> *) shared-gc
>> - This is mostly boilerplate that is common to any GC
>> - referenceProcessor.cpp has a little change to make one assert not fail
>> (next to CMS and G1)
>> - taskqueue.hpp has some small adjustments to enable subclassing
>
> I've reviewed the shared-gc webrev. I only found a few trivial nits.
>
> ------------------------------------------------------------------------------
>
> src/hotspot/share/gc/shared/gcName.hpp
> 42 NA,
> 43 Shenandoah,
>
> Putting Shenandoah after NA seems odd.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/gcConfig.cpp
> 63 CMSGC_ONLY(static CMSArguments cmsArguments;)
> ...
> 69 SHENANDOAHGC_ONLY(static ShenandoahArguments shenandoahArguments;)
>
> Code alignment should probably be updated.
>
> Similarly here:
> 73 static const SupportedGC SupportedGCs[] = {
> ...
> 79 SHENANDOAHGC_ONLY_ARG(SupportedGC(UseShenandoahGC,
> CollectedHeap::Shenandoah, shenandoahArguments, "shenandoah gc"))
>
> and here:
> 97 void GCConfig::fail_if_unsupported_gc_is_selected() {
> ...
> 105 NOT_SHENANDOAHGC(FAIL_IF_SELECTED(UseShenandoahGC, true));
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/collectedHeap.hpp
> 92 // ShenandoahHeap
>
> Moving it after ParallelScavengeHeap would give a better order.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/barrierSetConfig.hpp
> 36 SHENANDOAHGC_ONLY(f(Shenandoah))
>
> Why is this "Shenandoah" while all the others are "<something>BarrierSet"?
> Don't know. I'll change it to be consistent. I have proposed a patch with all changes to shenandoah-dev: http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008420.html Will post an updated upstreaming webrev soon. Thanks for reviewing, Roman
signature.asc
Description: OpenPGP digital signature
