RFR(S): 8185694: Replace SystemDictionaryShared::_java_platform_loader with SystemDictionary::is_platform_class_loader()

[David Holmes]

On 6/10/2017 3:40 PM, Calvin Cheung wrote:
> On 10/5/17, 6:43 PM, David Holmes wrote:
>> On 6/10/2017 10:59 AM, Calvin Cheung wrote:
>>> updated webrev: http://cr.openjdk.java.net/~ccheung/8185694/webrev.03/
>>
>> src/hotspot/share/classfile/systemDictionary.hpp
>>
>> // Returns default system loader
>>
>> Actually returns the system loader whether default or not.
> I'll update the comment.
>> Unclear if you might actually want two queries: one for default and 
>> one for actual (which has the default as its parent)
> I can file another bug if we need the default one.
>>
>> // Returns default platform loader
>>
>> You don't need to say "default" as there is only ever one platform 
>> loader.
> I'll update the comment.
>>
>> ---
>>
>> share/classfile/systemDictionary.cpp
>>
>>  186   return (class_loader == _java_system_loader);
>>
>> I think you need to restore this to checking both the field or whether 
>> an instance of the default system loader class - as per previous email 
>> - to handle the custom system loader case.
> I don't mind restoring the check but I've run many tests including most 
> of the ones mentioned in your email and didn't see any failures.

As Coleen indicated we don't really seem to test this area extensively. 
But again I'd have to look at all the code paths to see where we might 
have exposure to problems.

>>
>> The explicit NULL checks will be needed if these can be called before 
>> the platform/system loaders have been determined. I suspect they can 
>> given the complex initialization process we have with modules.
> So far I haven't run into any issues yet.

With the NULL checks removed? IF so, same answer as above.

Thanks,
David

> thanks,
> Calvin
>>
>> Thanks,
>> David