Thanks everyone, this work has now been committed:
author dholmes
Sun Jan 13 19:57:06 2013 -0500 (5 minutes ago)
changeset 6357 1109bfff4e92
parent 6356 7db04ae3378f
8005232: (JEP-149) Class Instance size reduction
Summary: Moved the fields for cached reflection objects into a seperate
ReflectionData object to reduce dynamic footprint.
Reviewed-by: dholmes, mchung, shade
Contributed-by: Peter Levart <peter.lev...@gmail.com>
David
-----
On 10/01/2013 4:59 PM, David Holmes wrote:
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
