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

mandy chung mandy.chung at oracle.com
Wed Dec 6 16:14:38 UTC 2017


Hi Andrej,

Thanks for the suggestion.  This code does not have to be performance 
sensitive but I will look at it and clean it up.

Mandy

On 12/6/17 2:08 AM, Andrej Golovnin wrote:
> 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