8173393: Module system implementation refresh (2/2017)

[Paul Sandoz]

Hi,

Just minor stuff in the JDK area (i browsed quickly through the tests).

Paul.


java.lang.module.Configuration
—

 321      * @implNote In the implementation then observability of modules may depend

s/then/the


ModuleDescriptor
—

2662     private static <T extends Object & Comparable<? super T>>
2663     int compare(Set<T> s1, Set<T> s2) {
2664         Iterator<T> iterator1 = new TreeSet<>(s1).iterator();
2665         Iterator<T> iterator2 =  new TreeSet<>(s2).iterator();
2666         while (iterator1.hasNext()) {
2667             if (!iterator2.hasNext())
2668                 return 1; // s1 has more elements
2669             T e1 = iterator1.next();
2670             T e2 = iterator2.next();
2671             int c = e1.compareTo(e2);
2672             if (c != 0)
2673                 return c;
2674         }
2675         if (iterator2.hasNext()) {
2676             return -1;  // s2 has more elements
2677         } else {
2678             return 0;
2679         }
2680     }

Potential optimisation. Convert the sets to arrays, sort ‘em, then use Arrays.compare.


BuiltinClassLoader
—

 925     private ModuleReader moduleReaderFor(ModuleReference mref) {
 926         return moduleToReader.computeIfAbsent(mref, m -> createModuleReader(m));
 927     }

Use this:: createModuleReader


jdk.internal.module.Builder
—

 279         Set<ModuleDescriptor.Modifier> modifiers;
 280         if (open || synthetic) {
 281             modifiers = new HashSet<>();
 282             if (open) modifiers.add(ModuleDescriptor.Modifier.OPEN);
 283             if (synthetic) modifiers.add(ModuleDescriptor.Modifier.SYNTHETIC);
 284             modifiers = Collections.unmodifiableSet(modifiers);
 285         } else {
 286             modifiers = Collections.emptySet();
 287         }

It would be nice to use the small collection methods given the sizes. The following might work:

  Set<ModuleDescriptor.Modifier> modifiers;
  int n = (open ? 1 : 0) + (synthetic ? 1 : 0);
  if (n > 0) {
      ModuleDescriptor.Modifier[] ms = new ModuleDescriptor.Modifier[n];
      if (open)
          ms[—n] = ModuleDescriptor.Modifier.OPEN;
      if (synthetic)
          ms[—n] = ModuleDescriptor.Modifier.SYNTHETIC;
      modifiers = Set.of(ms);
  } else {
      modifiers = Set.of();
  }


test/tools/jar/modularJar/Basic.java
—

 331     @Test(enabled = false)
 332     public void partialUpdateFooMainClass() throws IOException {

Just checking if that test still needs to be disabled.



—

Separately, we missed the opportunity to add lexicographical comparison and mismatch to List (we added them for arrays).

Paul.

> On 7 Feb 2017, at 14:48, Lois Foltan <lois.foltan at oracle.com> wrote:
> 
> Hotspot changes look good to me.
> Lois
> 
> On 2/7/2017 8:23 AM, Alan Bateman wrote:
>> We've been accumulating changes in the jake forest for the last few weeks and it's time to bring the changes to jdk9/dev, to make jdk-9+157 if possible. JDK 9 is the first phase of rampdown and so this update will need to get approval via the FC-extension process.
>> 
>> The changes this time are significantly smaller than previous updates. Much of this update is API and javadoc cleanup. There are a few new methods, and a few methods have been renamed/changed, but it's otherwise small potatoes.
>> 
>> The webrevs with the changes are here:
>>   http://cr.openjdk.java.net/~alanb/8173393/1/
>> 
>> Note that the webrevs are against jdk-9+155 because that is what jake is currently at. I will re-generate the webrevs later in the week once jdk-9+156 is promoted before eventually merging with jdk9/dev in advance of the eventual push.
>> 
>> One other thing to note is that the hotspot+jdk repos have the changes for JDK-8171855, this is only because that change was backed up in jdk9/hs for several weeks and only got to jdk9/dev for jdk-9+156. This means that the only non-test change in the hotspot repo is to methodHandles.cpp.
>> 
>> In the langtools repo then most of the changes are mechanical, with the only real update being cleanup to jdeps and the change to javac + javap to use the right values for the requires flags (the issue that Rémi brought up on jigsaw-dev last week when sync'ing up ASM).
>> 
>> In the jdk repo then ignore the DEBUG_ADD_OPENS changes in ModuleBootstrap, that is not intended for JDK 9.
>> 
>> There are a few small bug fixes, and a few more startup improvements from Claes. There are a small number of tests that aren't in jake yet for this update but they should be before I refresh the webrev later in the week.
>> 
>> -Alan
>> 
>