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

[David Holmes]

Hi Aleksey,

Thanks for the feedback. Let me prefix this by saying that time is of 
the essence here so unless there is a major issue things will go in as 
is, with follow ups for after M6 if needed. We can't miss M6 but we can 
tweak the initial changes after M6.

On 9/01/2013 10:19 PM, Aleksey Shipilev wrote:
> Good stuff.
>
> On 01/07/2013 02:46 AM, David Holmes wrote:
>> http://cr.openjdk.java.net/~dholmes/8005232/webrev/
>
> Sorry for obnoxious first-time reviewer questions:

No problem, I prepared suitably dismissive responses ;-)

>   a) I think the CAS loop in newReflectionData() is misleading in its
> reuse of the parameters. Can we instead inline it? Or, can we read the
> fields for $reflectionData and $classRedefinedCount, with no parameters
> passed?

As Peter commented this was out-lined for performance reasons. I can see 
your point over the reuse of the parameters as local variables but I 
don't have a problem with it. Arguably we save a couple of volatile 
field reads on the fastpath.

>   b) Had we considered filling up the complete ReflectionData eagerly on
> first access? This will make ReflectionData completely final. I would
> guess this goes against the per-use laziness?

If we did that then every reflection using class would use more memory - 
which defeats the whole point of this memory reduction exercise. ???

>   c) What's the merit of using Unsafe instead of field updaters here?
> (Avoiding the dependency on j.u.c?)

Yes, as Peter said, initialization order bites you.

>   d) I think the code can be further simplified if we stick with
> reflectionData() returning non-null object all the time, and check for
> useCaches instead in the usages, at the expense of creating the methods
> which actually get the reflection data.

I'm not quite sure what you mean, but assuming these are just 
stylistically different, there's no real motivation to make such a change.

>   e) Should useCaches be final? That will allow aggressive optimizations
> for (c).

It can't be final as it is controlled via a property query that can't be 
run during static initialization.

>> This is a saving of 7 reference fields ie. 28 bytes, resulting in a new
>> Class instance size of 80 bytes. This saves a further 4 bytes due to the
>> fields being 8-byte aligned without any need for padding. So overall we
>> save 32 bytes per class instance.
>
> Shameless promotion follows. This tool [1] should help to estimate the
> class layout in the face of object alignment, compressed references,
> paddings, alignment and whatnot.

I plan to take a look at this tool sometime - I promise. :)

Thanks,
David

> -Aleksey.
>
> [1] https://github.com/shipilev/java-object-layout/
>