RFR 8059357: ClassVerifier redundantly checks constant pool entries multiple times

[Harold Seigel]

Hi Ioi,

Thanks for looking at this!

See comments inline.

On 3/22/2019 1:41 PM, Ioi Lam wrote:
> 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?
If I call translate_signature() on demand then the get(sig_index) call 
will validly return NULL sometimes.
>
> 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?
Will do.
>
> 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.
Yes it is a good idea to call translate_signature() on demand.  I'll 
give it a try.  It will also allowing getting rid of method_sig_count().
>
> 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()?

I used a GrowableArray to take advantage of its append_if_missing() 
function but that won't be needed if done on demand.

Thanks! Harold

>
> 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
>>
>