RFR (S) 8222446: assert(C->env()->system_dictionary_modification_counter_changed()) failed: Must invalidate if TypeFuncs differ

[coleen.phillimore at oracle.com]

Dean,  Thank you for reviewing and for your help and discussion of this 
change.

On 6/21/19 3:48 PM, dean.long at oracle.com wrote:
> For the most part, this looks good.  I only have a couple concerns:
>
> 1) The distinction in both validate_compile_task_dependencies 
> functions between "dependencies failed" and "dependencies invalid" is 
> even more fuzzy after this change.  I suggest filing an RFE to remove 
> this distinction.

Yes, in jvmciRuntime I had to carefully preserve this logic or some 
tests failed.   I'll file an RFE for you.
>
> 2) In Parse::do_exits(), we don't know that concurrent class loading 
> didn't cause the problem.  We should be optimistic and allow the retry:
> C->record_failure(C2Compiler::retry_class_loading_during_parsing());
> rather than more drastic
>     C->record_method_not_compilable
> This is actually what the code did in an earlier revision.

Erik and I were trying to guess which was the right answer.  It seemed 
too lucky that you'd do concurrent class loading in this time period, so 
we picked the more drastic answer, but I tested both.  So I'll change it 
to the optimistic answer.

Thanks!
Coleen
>
> dl
>
> On 6/20/19 10:28 AM, coleen.phillimore at oracle.com wrote:
>> Summary: Remove SystemDictionary::modification_counter optimization
>>
>> See bug for more details.  To avoid the assert in the bug report, 
>> it's necessary to also increase the modification counter for class 
>> unloading, which needs special code for concurrent class unloading. 
>> The global counter is used to verify that validate_dependencies() 
>> gets the same answer based on the subklass hierarchy, but provides a 
>> quick exit in production mode.  Removing it may allow more nmethods 
>> to be created that don't depend on the classes that may be loaded 
>> while the Method is being compiled. Performance testing was done on 
>> this with no change in performance.  Also investigated the breakpoint 
>> setting code which incremented the modification counter. Dependent 
>> compilations are invalidated using evol_method dependencies, so 
>> updating the system dictionary modification counter isn't unnecessary.
>>
>> Tested with hs-tier1-8 testing, and CTW, and local jvmti/jdi/jdwp 
>> test runs with -Xcomp.
>>
>> open webrev at 
>> http://cr.openjdk.java.net/~coleenp/2019/8222446.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8222446
>>
>> Thanks,
>> Coleen
>