RFR: 8314986: Module readability resolution is slow with large numbers of automatic modules
Chen Liang
liach at openjdk.org
Tue Sep 26 15:01:17 UTC 2023
On Tue, 26 Sep 2023 14:19:55 GMT, Technici4n <duke at openjdk.org> wrote:
> Fixes the issue (hopefully) by resolving automatic modules and automatic module dependencies after propagation of non-automatic transitive dependencies. The module tests run.
>
> I also added a few asserts to validate that the automatic modules don't read themselves (previously this was the case with > 1 automatic module). Should these asserts be added in more places or is this enough?
>
> Alan also mentioned a conflict with the spec, I'm not sure which spec is being referred to. The documentation of `ResolvedModule#reads` states `A possibly-empty unmodifiable set`, implying that the set can be empty.
>
> Finally, `Configuration#reads` states `// The sets stored in the graph are already immutable sets` but that does not seem to be true. Should something be done about this to limit allocation?
>
> Please let me know!
> Cheers
src/java.base/share/classes/java/lang/module/Resolver.java line 1:
> 1: /*
You should update the ending copyright year of this file from 2022 to 2023.
src/java.base/share/classes/java/lang/module/Resolver.java line 621:
> 619:
> 620: String name = m1.name();
> 621: Set<ResolvedModule> m1Reads = entry.getValue();
Why not just this?
m1Reads.addAll(nameToResolved.values());
m1Reads.remove(m1);
src/java.base/share/classes/java/lang/module/Resolver.java line 639:
> 637: parent.configurations()
> 638: .map(Configuration::modules)
> 639: .flatMap(Set::stream)
Can't this and below step be `.forEach(m1Reads::addAll)`?
src/java.base/share/classes/java/lang/module/Resolver.java line 665:
> 663: } else {
> 664: // parent configuration, already resolved
> 665: // TODO: does this allocate a copy of the set every time?
It doesn't. `List.copyOf` and `Set.copyOf` returns the passed immutable collection instance if input is already an immutable collection.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337332185
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337342558
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337326631
PR Review Comment: https://git.openjdk.org/jdk/pull/15926#discussion_r1337350597
More information about the core-libs-dev
mailing list