RFR 8062180: MethodHandleImpl.makeArrays throws and swallows java.lang.NoSuchFieldError in normal flow

[Claes Redestad]

Hi Aleksey,

On 2014-11-04 22:25, Aleksey Shipilev wrote:
> Hi Claes,
>
> On 04.11.2014 23:57, Claes Redestad wrote:
>> please review this minor improvement to the initialization of
>> MethodHandleImpl$Lazy. Running any code which initializes
>> MethodHandlesImpl$Lazy will trigger throwing of
>> java.lang.NoSuchFieldErrors, which has a measurable impact on startup.
> Do you actually see the impact on startup? I thought the entire MHI$Lazy
> thing is here to avoid instantiation on startup. If that is not true,
> can you do a proper startup measurement on HelloWorld application?

I should have written nashorn/jjs startup or similar: any program
which initiates MethodHandlesImpl$Lazy. A simple java HelloWorld
wouldn't be affected, while bin/jjs hello.js will, for any choice of 
hello.js.

>
> Here is a handy one-liner:
>
> for S in `seq 1 1000`; do time java Hello; done 2>&1 | grep real | sed
> -e "s/0m//g" -e "s/s//g" | awk '{ sum += $2; n++ } END { if (n > 0)
> print sum / n; }'
>
>> Additionally, the current lookup via findCollector in varargsArray is
>> pointless (and will throw additional java.lang.NoSuchFieldErrors the
>> first time a varargsArray of arity > 11 is requested) since all static
>> methods which can be found will have been constructed already.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8062180
>> webrev: http://cr.openjdk.java.net/~redestad/8062180/webrev.00/
> Seems rather minor, but every little thing helps startup.

Yes and yes. :)

I could pitch this a cleanup effort as well, and would be happy to take 
this a
step further and throw exceptions rather than returning nulls from 
findCollector,
removing the assertions.

>
>   - Would we be better off asserting (mh == null) instead of breaking and
> asserting afterwards?

assert(mh != null) does makes it more readable...

>   - Would we save a few more bytecodes by using the method-local "final
> int ARRAYS_COUNT" to handle the magic number, instead of doing a
> full-blown field? Ditto for FILL_ARRAYS_COUNT.

... and renders ARRAYS_COUNT redundant.

The FILL_ARRAYS_COUNT is already used elsewhere, though.

New webrev: http://cr.openjdk.java.net/~redestad/8062180/webrev.01

>
>> A simple microbenchmark[1] show a very small (~0.5ms) improvement for
>> Class.forName("java.lang.invoke.MethodHandleImpl$Lazy")
>>
>> Benchmark                               Mode  Samples   Score Score
>> error  Units
>> o.s.CLBench.loadMethodHandleImplLazy ss      100  46.364
>> 1.740     ms
>> o.s.CLBench.loadMethodHandleImplLazy      ss      100 45.440
>> 1.515     ms
> Strictly speaking, there is no improvement. Run with more samples to
> shrink the error.

Ran with the updated code, 20k forks get the error down and establish
the improvement statistically:

Benchmark    Mode  Samples   Score  Score error  Units
loadMethodHandleImplLazy      ss    20000  40.257        0.102 ms
loadMethodHandleImplLazy      ss    20000  39.340        0.095 ms

Thanks!

/Claes

>
> -Aleksey.
>