RFR: 8155678: ClassLoader::initialize_module_loader_map should only be called when dumping CDS archive

[Calvin Cheung]

On 5/3/16, 6:16 PM, Jiangli Zhou wrote:
> Hi Calvin,
>
>> On May 3, 2016, at 4:42 PM, Jiangli Zhou<jiangli.zhou at Oracle.COM>  wrote:
>>
>> Hi Calvin,
>>
>>> On May 3, 2016, at 4:32 PM, Calvin Cheung<calvin.cheung at oracle.com>  wrote:
>>>
>>> Hi Jiangli,
>>>
>>> Just a minor comment in classLoaderExt.hpp:
>>>
>>> 58           jshort classloader_type = ClassLoader::classloader_type(
>>>
>>> How about u2 instead of jshort?
>> I agree. u2 is a better choice here. Will change it.
> I spoke too soon. InstanceKlass::set_class_loader_type() sets the loader type bits in '_misc_flags’ based on the ‘loader_type’ argument value. InstanceKlass has a method named loader_type(), which returns a u2 value. Note the u2 value returned by InstanceKlass::loader_type() is different from the ClassLoader::ClassLoaderType. The name of the InstanceKlass::loader_type() is missing leading, it’s better to be renamed. I’ll handle that separately.
I forgot that ClassLoader::classloader_type() returns a s2. We can 
change it to return a u2 but it can be done in a separate cleanup.
>
> ‘jshort’ is the same a ‘s2’’. To be consistent, I changed above ‘jshort’ to be ‘s2’, since ClassLoader.classloader_type() returns s2.
>
> Here is the updated webrev: http://cr.openjdk.java.net/~jiangli/8155678/webrev.01/
Looks good to me.

thanks,
Calvin
>
> Thanks,
> Jiangli
>
>> Thanks,
>> Jiangli
>>
>>> It is because in set_class_loader_type(), we are setting the _misc_flags which is of type u2.
>>>
>>> If you make the above change, you'll need to change set_class_loader_type() as well.
>>>
>>> thanks,
>>> Calvin
>>>
>>> On 5/3/16, 3:59 PM, Jiangli Zhou wrote:
>>>> Please review the following changes that make ClassLoader::initialize_module_loader_map() CDS dump time only. The module to loader map is not used by CDS runtime, so this avoids the unnecessary overhead for runtime. As part of the changes, I moved the ClassLoader::class loader_type() call from ClassLoader::load_class() into ClassLoaderExt::Context::record_result(), since ‘class loader_type’ is only needed by record_result(). That allows ClassLoader::class loader_type() become a CDS only API.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~jiangli/8155678/webrev.00/
>>>> bug: JDK-8155678<https://bugs.openjdk.java.net/browse/JDK-8155678>
>>>>
>>>> Thanks,
>>>> Jiangli