RFR (S) 8222446: assert(C->env()->system_dictionary_modification_counter_changed()) failed: Must invalidate if TypeFuncs differ
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Mon Jul 8 21:19:12 UTC 2019
Hi, From offline discussions, I updated the code in Parse::do_exits()
to make the method not compilable if the return types don't match.
Otherwise it would revert a change that Volker made to prevent infinite
compilation loops. It seems that the compiler code has been changed to
no longer exercise this path (ShouldNotReachHere never reached), so
keeping the conservative path seemed safest.
open webrev at http://cr.openjdk.java.net/~coleenp/2019/8222446.02/webrev
I changed the comment Dean, it might need help rewording.
Tested with tier1-8.
Thanks,
Coleen
On 6/21/19 4:44 PM, coleen.phillimore at oracle.com wrote:
>
> 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
>>
>
More information about the hotspot-dev
mailing list