On Fri, 25 Oct 2024 18:14:38 GMT, Ashutosh Mehra <[email protected]> wrote:
>> src/hotspot/share/classfile/systemDictionaryShared.cpp line 685:
>>
>>> 683: InstanceKlass* ik = InstanceKlass::cast(k);
>>> 684:
>>> 685: if (SafepointSynchronize::is_at_safepoint()) {
>>
>> Why is this piece of block required?
>> It calls `is_excluded_class` which reads `DumpTimeClassInfo::_excluded`
>> without checking for `has_checked_exclusion`. That means it can return false
>> (the default value) even for classes that may later be marked for exclusion
>> by `check_for_exclusion(ik, p)`.
>> <del>On the same note, I think we should add an assert in
>> `DumpTimeClassInfo::is_excluded` that `has_checked_exclusion()` is true.<del>
>
> Does the check `SafepointSynchronize::is_at_safepoint` imply that exclusion
> checks for all classes have already been done?
I renamed the public function to `should_be_excluded(InstanceKlass*)` to avoid
confusion with the private function `check_for_exclusion(InstanceKlass*,
DumpTimeClassInfo*)`. I also added code to make sure that exclusion has been
checked for the class, even when in safepoint.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1823688154
