RFR 8059357: ClassVerifier redundantly checks constant pool entries multiple times

[Ioi Lam]

Hi Harold,

This looks good overall. Thanks for making the verifier faster!

Some comments:

2831   sig_as_verification_types* mth_sig_verif_types = 
*(method_signatures_table()->get(sig_index));

How about calling the "get" method in a helper function, which asserts 
that the returned value is not NULL?

2999     for (int i = nargs; i < sig_verif_types_len; i++) {
3000       current_frame->push_stack(sig_verif_types->at(i), 
CHECK_VERIFY(this));

For added safety, how about asserting that the second item, if exists, 
must be ITEM_Long_2nd or ITEM_Double_2nd?

Also, is there any reason why ClassVerifier::init_method_sigs_table must 
be done at the beginning? Is it possible to do the translate_signature() 
on demand? That way you'd save one loop overate each element in the 
constant pool.

If you decide to keep the separate loop, then maybe unique_sig_indexes 
doesn't need to be a growable array, because you already counted the 
_klass->method_sig_count()?

Thanks
- Ioi

On 3/22/19 7:43 AM, Harold Seigel wrote:
> Hi,
>
> Please review this fix for JDK-8059357.  This fix improves how the 
> verifier translates method signatures into verification types. 
> Previously, the verifier did a separate signature translation for 
> every invoke* instruction even if the method signature was the same as 
> a previously translated signature.  With this change, the verifier, at 
> the beginning of class verification, loops through the class's 
> constant pool and translates each of the class's unique method 
> signatures just once.  Each signature's resulting verification types 
> are stored as an entry in a hash table indexed by the signature's 
> method_ref's Utf8 constant pool index.
>
> When verifying an invoke* instruction, the list of verification types 
> is retrieved from the hash table and stack argument assignability is 
> checked without having to translate the signature into its 
> verification types.  Return types are handled similarly.
>
> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8059357/webrev/
>
> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8059357
>
> The fix was regression tested by running Mach5 tiers 1 and 2 tests and 
> builds on Linux-x64, Windows, and Mac OS X, Mach5 tiers 3 -5 on 
> Linux-x64, and by running JCK-13 Lang and VM tests on Linux-x64.
>
> (Thanks to Eric Caspole for running performance tests on this change.)
>
> Thanks, Harold
>