RFR: 8259839 SystemDictionary exports too much implementation [v6]

Coleen Phillimore coleenp at openjdk.java.net
Tue Feb 2 12:24:43 UTC 2021


On Tue, 2 Feb 2021 02:36:34 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix more comments.
>
> src/hotspot/share/classfile/systemDictionary.cpp line 784:
> 
>> 782:             throw_circularity_error = true;
>> 783:           } else {
>> 784:             // case 3: traditional: should never see load_in_progress.
> 
> It seems odd to hit case 3 first in the code. I think the comment ordering of the cases should be retained.

I seriously disliked the comment case ordering.  It started with a special case and finally hit the general case. This is why I've never read this comment before or it never made any sense to me. Now it does.

> src/hotspot/share/classfile/systemDictionary.cpp line 792:
> 
>> 790:                 SystemDictionary_lock->wait();
>> 791:               } else {
>> 792:               // case 3: traditional with broken classloader lock. wait on first
> 
> Why combine these into case 3 instead of keeping two distinct cases as per the original comment?

I could do this.  I could add case 4 for the the broken class_loader lock.  or case 3.1.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2247


More information about the hotspot-runtime-dev mailing list