On Thu, 19 Sep 2024 04:00:38 GMT, Ioi Lam <[email protected]> wrote:
>> src/hotspot/share/cds/aotClassLinker.cpp line 145:
>>
>>> 143: return false;
>>> 144: }
>>> 145: }
>>
>> Are we concerned with the possibility that we might be able to add some
>> interfaces but not all, hence returning false, but with a subset of
>> interfaces already added to the candidate list? I don't think it should be
>> possible, but the code structure makes it look like it could be possible.
>
> That's possible. Since we go through all the initial set of classes found in
> `ArchiveBuilder::current()->klasses()`, eventually all classes that are
> eligible for aot-linking will be added to the candidate list. The reason for
> the recursion is to
>
> - Filter out classes whose super types are not eligible
> - Sort the candidate list so that super types always come before sub types.
Doesn't that potentially leave a "super" class that has no subclasses? Or do we
not really care?
>> src/hotspot/share/cds/aotClassLinker.cpp line 191:
>>
>>> 189: if (ik->class_loader() != class_loader) {
>>> 190: continue;
>>> 191: }
>>
>> This seems very inefficient. We call `write_classes` 4 times, potentially
>> with different loaders. Because the candidates are sorted the classes
>> belonging to the same loader are likely to be grouped due to package names.
>> So the app loader classes are likely to be right at end, and we have to
>> traverse all the boot/platform classes first before we get to them.
>> Conversely after we have encountered the last boot loader class (for
>> example) we keep the scanning the entire list. If the set were ordered based
>> on loader then name, we would be able to stop once we see the loader change
>> to not being the desired one. And a binaery search would let you find the
>> start of a section more quickly.
>
> The classes are sorted by class hierarchy. It's a linear operation repeated 4
> times, so it's not really something we need to optimize.
Okay so not sorted in any way that I anticipated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766187802
PR Review Comment: https://git.openjdk.org/jdk/pull/20843#discussion_r1766188844
