RFR (S): 8184667: Clean up G1ConcurrentMark files
Thomas Schatzl
thomas.schatzl at oracle.com
Fri Oct 20 12:12:31 UTC 2017
Hi Stefan,
thanks for your review.
On Fri, 2017-10-20 at 14:04 +0200, Stefan Johansson wrote:
> Thanks for cleaning up this code Thomas,
>
> On 2017-10-19 15:44, Thomas Schatzl wrote:
> > Hi all,
> >
> > can I have reviews for this code cleanup of G1ConcurrentMark*
> > files?
> >
> > The idea is to fix formatting, naming, assert strings, remove
> > unused
> > variables and methods, some missing braces around statements, and
> > look
> > into whether member visibility could be improved.
> >
> > No actual behavior changes.
> >
> > The CR contains references to a few other cleanup RFEs that I
> > thought
> > would not fit perfectly.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8184667
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8184667/webrev/
>
> Looks good in general. Just a few small things:
>
> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp:
> 1028 G1CMConcurrentMarkingTask markingTask(this, cm_thread());
>
> Rename markingTask to either just task or marking_task.
> ---
> src/hotspot/share/gc/g1/g1ConcurrentMark.hpp:
> 638 #ifdef ASSERT
> 639 // The task queue set needed for stealing
> 640 G1CMTaskQueueSet* _task_queues;
> 641 // Indicates whether the task has been claimed
>
> 642 bool _claimed;
> 643 #endif // ASSERT
>
> I wouldn't mind if you just removed those two. Not sure keeping them
> around is worth it.
> ---
>
I agree. All fixed in
http://cr.openjdk.java.net/~tschatzl/8184667/webrev.1 (full)
http://cr.openjdk.java.net/~tschatzl/8184667/webrev.0_to_1 (diff)
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list