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

[dean.long at oracle.com]

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.

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.

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