On Mon, 26 Jan 2026 13:49:49 GMT, Eirik Bjørsnøs <[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 >> using a directory class path) >> * 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. > > Eirik Bjørsnøs has updated the pull request incrementally with three > additional commits since the last revision: > > - Be more specific in comment about rejecting null URL elements > - Add periods to separate sentences in field comments > - Rename expandedPath to manifestClassPath test/jdk/jdk/internal/loader/URLClassPath/JarClassPathDfsOrder.java line 46: > 44: /* > 45: * @test > 46: * @bug 8375580 The OpenJDK dev guide has been updated to clarify when to use the `@bug` on tests https://openjdk.org/guide/. As per the guideline: > These bug ids refer to product bugs for which a fix is verified by this test. > JBS issues that track changes to the test itself are not listed here. This wasn't a product bug, so the `@bug` isn't necessary for this new test. test/jdk/jdk/internal/loader/URLClassPath/JarClassPathDfsOrder.java line 50: > 48: * @run junit JarClassPathDfsOrder > 49: */ > 50: public class JarClassPathDfsOrder { The JAR specification for the `Class-Path` attribute states: > The resulting URLs are inserted into the class path, immediately following > the URL of the context JAR. For example, given the following class path ... so it's good to introduce a test for this (I haven't verified we don't have any). However, I think the use of DFS in the test name and comments can be removed. I think we should name the test `JarManifestClassPathOrder`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29288#discussion_r2727708745 PR Review Comment: https://git.openjdk.org/jdk/pull/29288#discussion_r2727701607
