Final RFR: 8005232 (JEP-149) Class Instance size reduction

[David Holmes]

Hi Mandy,

Thanks for the review.

On 10/01/2013 11:25 AM, Mandy Chung wrote:
> On 1/6/2013 2:46 PM, David Holmes wrote:
>>
>> http://cr.openjdk.java.net/~dholmes/8005232/webrev/
>
> This looks good to me. David - besides the footprint performance data,
> do you observe any performance difference in your performance testing
> (I think refworkload is what you ran)?

I added the performance numbers to the bug report:

https://jbs.oracle.com/bugs/browse/JDK-8005232

No observable performance differences with refworkload.

>
> On 01/09/2013 01:19 PM, Aleksey Shipilev wrote:
>> On 01/09/2013 05:04 PM, Peter Levart wrote:
>>
>>> Strictly speaking, CAS is actually not needed here. Since we keep
>>> classRedefinedCount snapshot inside the ReflectionData, any races that
>>> would install "older" ReflectionData over "newer", would be quickly
>>> caught at next invocation to reflectionData(). So if there are any
>>> objections to usage of Unsafe, it can be removed and replaced by simple
>>> volatile write.
>>>
>> Yes, I think that would be more clear without any adverse impacts on
>> performance. Also, that better expresses the intent of requiring the
>> visibility, not the atomicity of cache update.
>
> I agree with Alekesey that it'd be simpler and clearer if CAS is replaced
> with the simple volatile write. But this change will require another round
> of performance measurement to determine its impact. I'd suggest to follow
> up this cleanup in a separate bug and changeset.

I'm not sure about the validity of changing from CAS to a simpe write - 
would need to think more deeply on it. But I agree with Mandy that the 
performance impact would need to be investigated. In the normal case it 
would be faster with no CAS but if there were "collisions" that might 
change. Anyway something to consider for a follow-up.

Thanks again,
David

> Mandy