Review Request JDK-8192945: Need stable sort for MODULES entry in the release file

Andrej Golovnin andrej.golovnin at gmail.com
Wed Dec 6 10:08:13 UTC 2017


Hi Mandy,

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ModuleSorter.java

 105         Deque<ResourcePoolModule> nodes = new LinkedList<>();
 106         graph.keySet().stream()
 107              .sorted(Comparator.comparing(ResourcePoolModule::name))
 108              .forEach(nodes::add);

It is suboptimal from the performance standpoint of view. Simpler and better:

List<ResourcePoolModule> nodes = new ArrayList<>(graph.keySet());
nodes.sort(Comparator.comparing(ResourcePoolModule::name));

But maybe you can solve the sorting problem just by using the TreeMap
for the graph variable in the line 51 and the TreeSet in the method
addNode(ResourcePoolModule).

A little bit unrelated to your changes:

 110         Deque<ResourcePoolModule> visited = new LinkedList<>();
 111         Deque<ResourcePoolModule> done = new LinkedList<>();

visited and done should be of type Set<ResourcePoolModule> to avoid
linear searching in the lines 114, 128, 129.

Best regards,
Andrej Golovnin

On Tue, Dec 5, 2017 at 11:19 PM, mandy chung <mandy.chung at oracle.com> wrote:
> The MODULES list in the `release` file is topologically sorted.
> For a given module graph, the patch traverses the graph in a
> stable order during the topological sort that will produce
> the same result for the same module graph.
>
> Webrev at:
> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8192945/webrev.00/
>
> Thanks
> Mandy


More information about the jigsaw-dev mailing list