On Fri, 25 Oct 2024 18:08:17 GMT, Ashutosh Mehra <asme...@openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - 8342907: Implement AOT testing mode for jtreg tests (authored by
>> @katyapav)
>> - disable test that fails with hotspot_runtime_non_cds_mode
>
> 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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1817167862
