RFR (XXS): 8040792: G1CodeRootChunkManager::static_mem_size returns the wrong size

[Mikael Gerdin]

Hi Thomas,

On Friday 18 April 2014 15.29.42 Thomas Schatzl wrote:
> Hi Bengt,
> 
>   thanks for the review.
> 
> On Thu, 2014-04-17 at 13:36 +0200, Bengt Rutisson wrote:
> > Hi Thomas,
> > 
> > On 2014-04-17 12:42, Thomas Schatzl wrote:
> > > Hi all,
> > > 
> > > to
> > > 
> > >    87   return sizeof(*this);
> > 
> > I would kind of prefer
> > 
> >   return sizeof(G1CodeRootChunkManager);
> > 
> > since it would be faster for me to read.
> 
> Done. I also changed a few "sizeof(*this)" at the same time.
> 
> > Also, there are more places in the G1 code where we do sizeof(this). At
> > a first glance it looks to me like all of them are wrong.
> > 
> > $ grep -r "sizeof(this)" src/share/vm/gc_implementation/g1/
> > src/share/vm/gc_implementation/g1/sparsePRT.cpp:  return sizeof(this) +
> > src/share/vm/gc_implementation/g1/sparsePRT.cpp:  return sizeof(this) +
> > _next->mem_size();
> > src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp:  return
> > sizeof(this);
> > src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp:  return
> > sizeof(this) + _list.count() * _list.size();
> > src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp:      +
> > (sizeof(this) - sizeof(OtherRegionsTable))
> > src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp:    return
> > sizeof(this) + _bm.size_in_words() * HeapWordSize;
> > 
> > Can you verify these at the same time?
> 
> Fixed all of them.
> 
> I only added test cases where there is already infrastructure to do so
> for this trivial "> sizeof(void*)" checks, and where there was no need
> to do too much calculation.
> 
> I hope this is okay. If not, I prefer splitting these out into an extra
> CR.

I think it's fine to fix all these at the same time but please update the bug 
synopsis and description to reflect that all these should be fixed.

> 
> New webrev:
> http://cr.openjdk.java.net/~tschatzl/8040792/webrev.1/

Looks good.

/Mikael

> 
> Testing:
> new test cases, jprt
> 
> Thanks,
>   Thomas