RFR: JDK-8263411: Convert jshell tool to use Stream.toList()

Jan Lahoda jlahoda at openjdk.java.net
Thu Mar 18 20:27:41 UTC 2021


On Mon, 15 Mar 2021 23:35:14 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> This converts jshell from using `Stream.collect(Collectiors.toList())` to `Stream.toList()` - an immutable list with better performance characteristics. Local inspection only turned up one place that was mutating a resulting list and that has been refactored. 
>> 
>> This work is a subtask to: https://bugs.openjdk.java.net/browse/JDK-8260559
>
> src/jdk.jshell/share/classes/jdk/jshell/SnippetMaps.java line 177:
> 
>> 175:                                .map(isi -> isi.fullname.substring(0, isi.fullname.lastIndexOf(".")));
>> 176:         pkgs = Stream.concat(Stream.of("java.lang"), pkgs);
>> 177:         for (String ipkg : pkgs.toList()) {
> 
> The original code is strictly speaking in error, as it's modifying the list returned from `Collectors.toList()` which is not guaranteed to be mutable. (Obviously, this isn't enforced.) Instead of concatenating the element to the head of the stream and using `Stream.toList()`, though, it might be just as easy to convert the original code to use `collect(Collectors.toCollection(ArrayList::new))` and leave the insertion of "java.lang" at the front at before. Might make things clearer as to what's going on.
> 
> (A slight variation would be to static-import j.u.s.Collectors.toCollection.)
> 
> I'll defer to the jshell team on their preference here.

If I was writing this, I'd probably go with `collect(Collectors.toCollection(ArrayList::new))`, but I don't see the `concat` as too bad. When using `concat`, another idea would be to avoid the `.toList()`, and re-write the loop to Streams completely. After all, it seems to be something along the lines of:
        Stream<String> pkgs = importSnippets()
                               .filter(isi -> isi.isStar)
                               .map(isi -> isi.fullname.substring(0, isi.fullname.lastIndexOf(".")));
        return Stream.concat(Stream.of("java.lang"), pkgs)
                     .filter(ipkg -> !ipkg.isEmpty() && ipkg.equals(pkg))
                     .map(ipkg -> full.substring(pkg.length() + 1))
                     .findFirst()
                     .orElse(full);

-------------

PR: https://git.openjdk.java.net/jdk/pull/2979


More information about the kulla-dev mailing list