On Mon, 19 Jan 2026 21:32:48 GMT, Claes Redestad <[email protected]> wrote:
>> Please review this PR which proposes we replace `j.u.ArrayDeque` with
>> `j.u.ArrayList` in `jdk.internal.loader.URLClassPath`.
>>
>> The motivation for using a double-ended queue may have been that "original"
>> search path URLs are added to the tail of the queue while any URLs
>> discovered during JAR `Class-Path` expansion are inserted at the head such
>> that they are processed first.
>>
>> By splitting these two concerns and processing loader discovered class path
>> URLs separately from the original search path, we no longer need a
>> double-ended queue. We can replace `ArrayDeque` with `ArrayList`.
>>
>> Advantages:
>>
>> * A "hello world" Java program no longer loads `ArrayDeque`, `Deque` and
>> `Queue` (`URLClassPath` is the only consumer of these classes during startup)
>> * One data structure is simpler than two
>> * We no longer need to manage search path URLs across two different
>> collections
>> * Code and comments to dance around `ArrayDeque` calling into lambda too
>> early is no longer a concern and can be removed
>> * I think this PR leaves the code overall simpler and easier to follow.
>>
>> Testing:
>>
>> This PR introduces a new test to verify that URLs disovered via a
>> multi-level tree paths discovered via `Class-Path` JAR attributes are found
>> in the expected DFS order. The ordering aspect seems to lack existing
>> coverage.
>
> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 212:
>
>> 210: return;
>> 211: synchronized (searchPath) {
>> 212: if (! searchPath.contains(url)) {
>
> Does it matter that this would now search through all opened URLs while
> before we'd only scan through the list of unopened? I guess you'd need
> something like `!searchPath.subList(nextURL, searchPath.size() +
> 1).contains(url)` to be perfectly neutral semantically here, but I'm not sure
> if re-adding previously opened URLs is a valid use-case.
Hello Claes!
> Does it matter that this would now search through all opened URLs while
> before we'd only scan through the list of unopened?
Not sure I follow. If by "scan through" you mean the `contains` call, then we
scan through the same list of URLs before and after this PR. It's the list of
URLs passed in the constructor, possibly amended by calls to `addURL`. It was
renamed from `path` to `searchPath`.
Whether URLs where processed/loaded did not matter before this PR, and it does
not matter now. Before we needed to add URLs to both `unopenedUrls` (to find it
during loading) and to `paths` (because we needed to return the amended list in
`getURLs`.
Perhaps I misunderstood what you're saying here.. Could you also re-read the
code and check that you got it right on your side?
We should definitely get to the bottom of this, we don't want to introduce any
behavior changes in this PR, should be a pure refactoring.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29288#discussion_r2706246854
