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

[Bengt Rutisson]

On 4/22/14 9:54 AM, Mikael Gerdin wrote:
> 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.

Looks good to me too. I agree with Mikael about updating the bug 
synopsis and description.

Thanks,
Bengt


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