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

Thomas Schatzl thomas.schatzl at oracle.com
Fri Apr 18 13:29:42 UTC 2014


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.

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

Testing:
new test cases, jprt

Thanks,
  Thomas





More information about the hotspot-gc-dev mailing list