[9] RFR (S): 8058892: FILL_ARRAYS and ARRAYS are eagely initialized in MethodHandleImpl

[Aleksey Shipilev]

On 10/02/2014 09:26 PM, Vladimir Ivanov wrote:
> Aleksey,
> 
> Thanks for the review.
> Updated version:
> http://cr.openjdk.java.net/~vlivanov/8058892/webrev.02/

Looks good.

>> * Since initialization order is important, why don't put the
>> initialization in the existing static initializer? This will secure for
>> inadvertent field reordering in future.
> Good idea. Fixed.
> 
>> * Any reason two new fields are "private"? All other seem
>> package-private.
> These fields are intended for usage only in
> MHI.varargsArray/buildFiller. They were private before and I decided to
> keep that. It is fine for MH_*/NF_* to be used from other places in
> j.l.i (e.g. NF_checkSpreadArgument is used in LambdaFormEditor).

Okay. My sentiment was on par with Remi's: needless access methods for
private fields. Now that these fields are in Lazy, the access methods
are generated when the fields are accessed from the enclosing class
(MethodHandleImpl).

>> * Any performance problems if we actually count FILL_ARRAYS_COUNT in
>> during the static initialization, instead of putting a magic value?
> It's more about code structure than performance.
> 
> There's a dependency between FILL_ARRAYS size & LEFT_ARGS.
> If FILL_ARRAYS_COUNT is moved to Lazy, LEFT_ARGS should be moved to Lazy
> as well or LEFT_ARGS should be lazily initialized separately.
> IMO it doesn't worth an effort.

I am not talking about moving FILL_ARRAYS_COUNT to Lazy. I am talking
about computing the FILL_ARRAYS_COUNT in the new static initializer of
MethodHandleImpl itself -- so that we would not get bugs when adding new
fillArray override. But, given there are asserts in the code, it seems
excessive. Leave it as is.

-Aleksey.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20141002/0eccf9d4/signature.asc>