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.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
