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.fol...@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
>>
>
