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