RFR: 8075829: Move CSpaceCounters implementation to cSpaceCounters.cpp

Bengt Rutisson bengt.rutisson at oracle.com
Wed Mar 25 08:43:25 UTC 2015


On 2015-03-25 09:43, Stefan Karlsson wrote:
> Hi Bengt,
>
> On 2015-03-25 09:19, Bengt Rutisson wrote:
>>
>> Hi Stefan,
>>
>> On 2015-03-24 17:33, Stefan Karlsson wrote:
>>> Hi,
>>>
>>> Please review this patch to reduce the dependency against 
>>> space.inline.hpp in cSpaceCounters.hpp.
>>>
>>> - The update_* functions are not called frequently and doesn't have 
>>> to be inlined.
>>
>> It looks to me like the update_* functions don't need to be virtual. 
>> Can we make them non-virtual?
>
> Good point, although not really related to my change. I took a look at 
> CSpaceCounter and you're right, the update functions are never 
> overridden. However, there's a similar class named GenerationCounters, 
> which has a virtual update_all function that is overridden by 
> subclasses. Let's change this in a separate RFE, if we think it's 
> worth changing.

OK. Sounds good.

Reviewed.

Bengt

>
>> Otherwise looks good.
>
> Thanks.
>
> StefanK
>
>>
>> Bengt
>>
>>>
>>> - The take_sample() function is called as a virtual call, and 
>>> doesn't need to be inlined.
>>>
>>> - The change in cSpaceCounters.hpp to include space.hpp instead of 
>>> space.inline.hpp, causes some files to no longer include 
>>> space.inline.hpp. If one of these files use the Space class but 
>>> doesn't include space.inline.hpp we get a compile error stating that 
>>> block_start is used but never defined, even when the block_start 
>>> function isn't used from that file. To solve that, the 'inline' 
>>> keyword is removed from the declaration, but kept at the definition. 
>>> The compiler will not complain about missing definition of 
>>> block_start if space.inline.hpp isn't included, but the linker will. 
>>> The compiler will still be able to inline the function.
>>>
>>> http://cr.openjdk.java.net/~stefank/8075829/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8075829
>>>
>>> Thanks,
>>> StefanK
>>
>




More information about the hotspot-gc-dev mailing list