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

[Calvin Cheung]

On 10/5/17, 11:05 PM, David Holmes wrote:
> 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.
That was with the NULL check.
Without the NULL check, the following test failed:
./hotspot/jtreg/runtime/modules/ModuleStress/ModuleStress.java

I'll restore SystemDictionary::is_system_class_loader() to its original 
form.
I'll send updated webrev after more testing.

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