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