RFR (M): 8184346: Clean up G1CMBitmap

[Thomas Schatzl]

Hi all,

  sorry for the long wait...

On Fri, 2017-07-21 at 10:17 +0200, Aleksey Shipilev wrote:
> On 07/21/2017 10:15 AM, Mikael Gerdin wrote:
> > 
> > On 2017-07-20 20:50, Aleksey Shipilev wrote:
> > > 
> > > On 07/20/2017 08:40 PM, Thomas Schatzl wrote:
> > > > 
> > > > Webrevs:
> > > > http://cr.openjdk.java.net/~tschatzl/8184346/webrev.2_to_3/
> > > > (diff)
> > > > http://cr.openjdk.java.net/~tschatzl/8184346/webrev.3/ (full)
> > Looks fine to me too.
> > 
> > > 
> > > Generally good, comments:
> > > 
> > >   *) Long log_debug, log_warning, assert lines in
> > > g1CollectedHeap.cpp,
> > > g1ConcurrentMark.cpp, g1ConcurrentMark.hpp
> > > 

I fixed the ones in g1ConcurrentMark*; not sure which assert that has
not been formatted already one has been changed in in
g1CollectedHeap.cpp.

> > >   *) It seems the field and method names are camel-cased and thus
> > > style-inconsistent with the rest of the code?
> > > 625   const G1CMBitMap* const prevMarkBitMap() const { return
> > > _prevMarkBitMap; }
> > > 626   G1CMBitMap* nextMarkBitMap() const { return
> > > _nextMarkBitMap; }
> > I think the idea is to perform that renaming in G1ConcurrentMark in
> > a later change since this one tries to only concern G1CMBitMap.
> No problem! Fix the long asserts, and I am happy with the patch.
> 

Yeah, there will be a renaming pass to g1ConcurrentMark* soon (and lots
more passes to fix various aspects). Let's keep the patches focused. :)

Webrevs:
http://cr.openjdk.java.net/~tschatzl/8184346/webrev.3_to_4/ (diff)
http://cr.openjdk.java.net/~tschatzl/8184346/webrev.4/ (full)

Thanks a lot,
  Thomas