RFR 8190235: Clarify ClassLoaderData::is_*_class_loader_data() method implementations

Volker Simonis volker.simonis at gmail.com
Tue Feb 6 18:49:01 UTC 2018


Hi Harold,

the change looks good, but why we now need two methods (i.e.
is_the_null_class_loader_data() and is_boot_class_loader_data()) for
the same thing?

In general, I more like the name is_boot_class_loader_data(), but
then, shouldn't we replace all the occurrences of
is_the_null_class_loader_data() with is_boot_class_loader_data().
Introducing is_boot_class_loader_data() just for using it a single
time seem weird to me, or did I missed something?

Regards,
Volker


On Tue, Feb 6, 2018 at 7:38 PM, harold seigel <harold.seigel at oracle.com> wrote:
> Hi Coleen,
>
> Thanks for reviewing this.
>
> Note that the rfr adds an assert to ModuleEntry::set_loader_data() to help
> ensure that a module's class loader data is not anonymous.
>
> Harold
>
>
>
> On 2/6/2018 1:35 PM, coleen.phillimore at oracle.com wrote:
>>
>>
>> This looks good.   I don't see any code that assumes
>> is_builtin_class_loader_data means the same thing as
>> is_permanent_class_loader_data() but there are a couple of instances in
>> ModuleEntry::set_read_walk_required and
>> PackageEntry::set_export_walk_required, but these might be okay because
>> there isn't a module reads list or exports list on anonymous CLD.
>> thanks,
>> Coleen
>>
>> On 2/5/18 3:37 PM, harold seigel wrote:
>>>
>>> Hi,
>>>
>>> Please review this small JDK-11 RFR to clarify and clean up the
>>> implementations of the ClassLoaderData::is_*_class_loader_data()
>>> implementations.  The changes primarily consist of adding some comments.
>>>
>>> Open Webrev:
>>> http://cr.openjdk.java.net/~hseigel/bug_8190235/webrev/index.html
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8190235
>>>
>>> The changes were tested with Mach 5 tier1 - tier5 tests and JTReg JDK
>>> tests.
>>>
>>> Thanks, Harold
>>>
>>
>


More information about the hotspot-runtime-dev mailing list