Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

[Alan Bateman]

On 11/10/2016 19:03, Mandy Chung wrote:


This patch updates jar, jlink, jmod tool to be a provider of the new tool SPI.  
Some tests are also updated to replace the use of internal APIs with 
ToolProvider::findFirst to look up a tool provider.  There are more tests that 
can be updated and something to be cleaned up in the future.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html

This update to the tools looks good. For the tests then 
ToolProvider.findFirst("jar").get() will draw the attention of the 
optional police and might be better to use orElseThrow to supply an 
exception that clearly indicates that the tool cannot be found.


-Alan

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

[Naoto Sato]

+1 for removing the link to Collections#unmodifiableList in the spec. I 
don't see any particular reason to specify in the javadoc and don't 
believe it would break any existing apps.


Naoto

On 10/11/16 4:03 PM, Stuart Marks wrote:



On 10/10/16 11:26 PM, Andrej Golovnin wrote:

src/java.base/share/classes/java/util/ResourceBundle.java

2490 public static class Control {
2491 /**
2492  * The default format List, which contains
the strings
2493  * "java.class" and
"java.properties", in
2494  * this order. This List is {@linkplain
2495  * Collections#unmodifiableList(List) unmodifiable}.
2496  *
2497  * @see #getFormats(String)
2498  */
2499 public static final List FORMAT_DEFAULT =
List.of("java.class", "java.properties");

I think you should also change the JavaDocs in the
ResourceBundle.Control class for the constants FORMAT_CLASS,
FORMAT_DEFAULT and FORMAT_PROPERTIES, because the JavaDocs for this
constants explicitly mentions, that the lists are created by using
Collections#unmodifiableList(List). Or you cannot change this
constants at all because they are part of the Public API and existed
in this form for a long time. Please ask someone from Oracle for help.
They can explain it better when it is OK to change and when not. Maybe
Stuart can do that.


Hi Andrej,

Thanks for pointing this out.

It appears that the changes to remove the links to
Collections.unmodifiableList() was dropped from webrev.01 to webrev.02.
I've restored them, along with a couple other changes that were also
dropped. I also updated the ModuleFinder.java comment per a request from
Alan Bateman. The revised webrev is here:

http://cr.openjdk.java.net/~smarks/reviews/8134373/webrev.03/

In any case, yes, the specifications of the ResourceBundle.Control
fields should be changed to remove the links to
Collections.unmodifiableList(). It's unclear whether having a link this
way implies that it's part of the specification that these fields must
be instances returned from that method. Removing the link makes it clear
that saying the List is unmodifiable is merely a description of the
required behavior.

I spoke with Joe Darcy (our compatibility guru) and we agreed that out
of an abundance of caution it would be wise to file a request to make
this change. (This is the "CCC" - basically an internal change control
process for Java SE specifications.) Doing this is mainly for tracking
purposes, as we believe this to be a compatible change.

I've also included in this request a mention of the change to
CookieManager.get() which we had discussed previously. Even though we
believe this is also a compatible change, it's also a change that should
be tracked.

I'll follow up when this bit of process is finished.

s'marks

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

[Stuart Marks]

On 10/10/16 11:26 PM, Andrej Golovnin wrote:

src/java.base/share/classes/java/util/ResourceBundle.java

2490 public static class Control {
2491 /**
2492  * The default format List, which contains the strings
2493  * "java.class" and "java.properties", in
2494  * this order. This List is {@linkplain
2495  * Collections#unmodifiableList(List) unmodifiable}.
2496  *
2497  * @see #getFormats(String)
2498  */
2499 public static final List FORMAT_DEFAULT =
List.of("java.class", "java.properties");

I think you should also change the JavaDocs in the
ResourceBundle.Control class for the constants FORMAT_CLASS,
FORMAT_DEFAULT and FORMAT_PROPERTIES, because the JavaDocs for this
constants explicitly mentions, that the lists are created by using
Collections#unmodifiableList(List). Or you cannot change this
constants at all because they are part of the Public API and existed
in this form for a long time. Please ask someone from Oracle for help.
They can explain it better when it is OK to change and when not. Maybe
Stuart can do that.


Hi Andrej,

Thanks for pointing this out.

It appears that the changes to remove the links to 
Collections.unmodifiableList() was dropped from webrev.01 to webrev.02. I've 
restored them, along with a couple other changes that were also dropped. I also 
updated the ModuleFinder.java comment per a request from Alan Bateman. The 
revised webrev is here:


http://cr.openjdk.java.net/~smarks/reviews/8134373/webrev.03/

In any case, yes, the specifications of the ResourceBundle.Control fields should 
be changed to remove the links to Collections.unmodifiableList(). It's unclear 
whether having a link this way implies that it's part of the specification that 
these fields must be instances returned from that method. Removing the link 
makes it clear that saying the List is unmodifiable is merely a description of 
the required behavior.


I spoke with Joe Darcy (our compatibility guru) and we agreed that out of an 
abundance of caution it would be wise to file a request to make this change. 
(This is the "CCC" - basically an internal change control process for Java SE 
specifications.) Doing this is mainly for tracking purposes, as we believe this 
to be a compatible change.


I've also included in this request a mention of the change to 
CookieManager.get() which we had discussed previously. Even though we believe 
this is also a compatible change, it's also a change that should be tracked.


I'll follow up when this bit of process is finished.

s'marks

Re: RFR (JAXP) 8152530: NullPointerException when xmlns=""

[Naoto Sato]

Thanks, Joe. +1

Naoto

On 10/11/16 2:32 PM, Joe Wang wrote:

Thanks Daniel, Naoto!

Changed to use prefix.isEmpty().

-Joe

On 10/11/16, 1:41 PM, Naoto Sato wrote:

Hi Joe,

You could use prefix.isEmpty() to check if it's an empty string.
Otherwise looks fine to me.

Naoto

On 10/11/16 1:29 PM, Joe Wang wrote:

Hi,

Please review a fix for a NPE when the source contains empty namespace.

JBS: https://bugs.openjdk.java.net/browse/JDK-8152530
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/

Thanks,
Joe

Re: RFR: 8062389, 8029459, 8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Paul Sandoz]

Hi Peter,

Nice work here.

PublicMethods
—

I would be inclined to change PublicMethods to encapsulate an instance of 
LinkedHashMap, since it’s only the consolidate/toArray methods that really 
matter, and no need for the key/value types to be exposed, then no need to 
declare serialVersionUID, and also it can be final. The JavaDoc on consolidate 
would need to be tweaked.

  29  * existing methods with same signature if they are less specific then 
added
s/then/than

  89 if (o == null || getClass() != o.getClass()) return false;

Could be simplified to

  if (!(o instanceof Key)) return false

Paul.

> On 4 Oct 2016, at 06:40, Peter Levart  wrote:
> 
> Hi,
> 
> I have a fix for conformance (P2) bug (8062389 
> ) that also fixes a 
> conformance (P3) bug (8029459 
> ) and a performance issue 
> (8061950 ):
> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/
> 
> As Daniel Smith writes in 8029459 
> , the following situation 
> is as expected:
> 
> interface I { void m(); }
> interface J extends I { void m(); }
> interface K extends J {}
> K.class.getMethods() = [ J.m ]
> 
> But the following has a different result although it should probably have the 
> same:
> 
> interface I { void m(); }
> interface J extends I { void m(); }
> interface K extends I, J {}
> K.class.getMethods() = [ I.m, J.m ]
> 
> He then suggests the following algorithm:
> 
> An implementation of getMethods consistent with JLS 8 would include the 
> following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
> 1) The class's/interface's declared (public) methods
> 2) The getMethods() of the superclass (if this is a class), minus any that 
> have a match in (1)
> 3) The getMethods() of each superinterface, minus any that have a match in 
> (1) or a _concrete_ match in (2) or a match from a more-specific 
> class/interface in (2) or (3)
> 
> But even that is not sufficient for the following situation:
> 
> interface E { void m(); }
> interface F extends E { void m(); }
> abstract class G implements E {}
> abstract class H extends G implements F {}
> H.class.getMethods() = [ E.m, F.m ]
> 
> The desired result of H.class.getMethods() = [ F.m ]
> 
> So a more precise definition is required which is implemented in the fix.
> 
> The getMethods() implementation partitions the union of the following methods:
> 
> 1) The class's/interface's declared public methods
> 2) The getMethods() of the superclass (if this is a class)
> 3) The non-static getMethods() of each direct superinterface
> 
> ...into equivalence classes (sets) of methods with same signature (return 
> type, name, parameter types). Within each such set, only the "most specific" 
> methods are kept and others are discarded. The union of the kept methods is 
> the result.
> 
> The "more specific" is defined as a partial order within a set of methods of 
> same signature:
> 
> Method A is more specific than method B if:
> - A is declared by a class and B is declared by an interface; or
> - A is declared by the same type as or a subtype of B's declaring type and 
> both are either declared by classes or both by interfaces (clearly if A and B 
> are declared by the same type and have the same signature, they are the same 
> method)
> 
> If A and B are distinct elements from the set of methods with same signature, 
> then either:
> - A is more specific than B; or
> - B is more specific than A; or
> - A and B are incomparable
> 
> A sub-set of "most specific" methods are the methods from the set where for 
> each such method M, there is no method N != M such that N is "more specific" 
> than M.
> 
> There can be more than one "most specific" method for a particular signature 
> as they can be inherited from multiple unrelated interfaces, but:
> - javac prevents compilation when among multiply inherited methods with same 
> signature there is at least one default method, so in practice, getMethods() 
> will only return multiple methods with same signature if they are abstract 
> interface methods. With one exception: bridge methods that are created by 
> javac for co-variant overrides are default methods in interfaces. For example:
> 
>interface I { Object m(); }
>interface J1 extends I { Map m(); }
>interface J2 extends I { HashMap m(); }
>interface K extends J1, J2 {}
> 
> K.class.getMethods() = [ default Object j1.m, abstract Map j1.m, default 
> Object j2.m, abstract HashMap j2.m ]
> 
> But this is an extreme case where one can still expect multiple non-abstract 
> methods with same signature, but different declaring type, returned from 
> getMethods().
> 
> In order to also fix 8062389 
> , getMethod() and 
> getMethods() share the same consolidation logic that results in a set of 
>

Re: RFR 8167524 Rogue character in Stream javadoc

[Stuart Marks]

-1 (character)

j/k, LGTM

On 10/11/16 2:30 PM, Joseph D. Darcy wrote:

+1

-Joe

On 10/11/2016 2:27 PM, Paul Sandoz wrote:

Hi,

I accidentally fat fingered the JavaDoc of Stream which got consumed into an
unrelated patch that i pushed [1]. Thanks go to Martin for noticing this.

Paul.

[1] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e93b7ea55975

diff -r b909daf8fdbc src/java.base/share/classes/java/util/stream/Stream.java
--- a/src/java.base/share/classes/java/util/stream/Stream.javaTue Oct 11
12:33:15 2016 +0200
+++ b/src/java.base/share/classes/java/util/stream/Stream.javaTue Oct 11
14:23:46 2016 -0700
@@ -282,7 +282,7 @@
   */
   Stream flatMap(Function>
mapper);

-/**:
+/**
   * Returns an {@code IntStream} consisting of the results of replacing 
each
   * element of this stream with the contents of a mapped stream produced by
   * applying the provided mapping function to each element.  Each mapped

Re: RFR (JAXP) 8152530: NullPointerException when xmlns=""

[Joe Wang]

Thanks Daniel, Naoto!

Changed to use prefix.isEmpty().

-Joe

On 10/11/16, 1:41 PM, Naoto Sato wrote:

Hi Joe,

You could use prefix.isEmpty() to check if it's an empty string. 
Otherwise looks fine to me.


Naoto

On 10/11/16 1:29 PM, Joe Wang wrote:

Hi,

Please review a fix for a NPE when the source contains empty namespace.

JBS: https://bugs.openjdk.java.net/browse/JDK-8152530
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/

Thanks,
Joe

Re: RFR 8167524 Rogue character in Stream javadoc

[Joseph D. Darcy]

+1

-Joe

On 10/11/2016 2:27 PM, Paul Sandoz wrote:

Hi,

I accidentally fat fingered the JavaDoc of Stream which got consumed into an 
unrelated patch that i pushed [1]. Thanks go to Martin for noticing this.

Paul.

[1] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e93b7ea55975

diff -r b909daf8fdbc src/java.base/share/classes/java/util/stream/Stream.java
--- a/src/java.base/share/classes/java/util/stream/Stream.java  Tue Oct 11 
12:33:15 2016 +0200
+++ b/src/java.base/share/classes/java/util/stream/Stream.java  Tue Oct 11 
14:23:46 2016 -0700
@@ -282,7 +282,7 @@
   */
   Stream flatMap(Function> 
mapper);

-/**:
+/**
   * Returns an {@code IntStream} consisting of the results of replacing 
each
   * element of this stream with the contents of a mapped stream produced by
   * applying the provided mapping function to each element.  Each mapped

RFR 8167524 Rogue character in Stream javadoc

[Paul Sandoz]

Hi,

I accidentally fat fingered the JavaDoc of Stream which got consumed into an 
unrelated patch that i pushed [1]. Thanks go to Martin for noticing this.

Paul.

[1] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e93b7ea55975

diff -r b909daf8fdbc src/java.base/share/classes/java/util/stream/Stream.java
--- a/src/java.base/share/classes/java/util/stream/Stream.java  Tue Oct 11 
12:33:15 2016 +0200
+++ b/src/java.base/share/classes/java/util/stream/Stream.java  Tue Oct 11 
14:23:46 2016 -0700
@@ -282,7 +282,7 @@
  */
  Stream flatMap(Function> 
mapper);

-/**:
+/**
  * Returns an {@code IntStream} consisting of the results of replacing each
  * element of this stream with the contents of a mapped stream produced by
  * applying the provided mapping function to each element.  Each mapped

Re: RFR (JAXP) 8152530: NullPointerException when xmlns=""

[Naoto Sato]

Hi Joe,

You could use prefix.isEmpty() to check if it's an empty string. 
Otherwise looks fine to me.


Naoto

On 10/11/16 1:29 PM, Joe Wang wrote:

Hi,

Please review a fix for a NPE when the source contains empty namespace.

JBS: https://bugs.openjdk.java.net/browse/JDK-8152530
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/

Thanks,
Joe

Re: RFR (JAXP) 8152530: NullPointerException when xmlns=""

[Daniel Fuchs]

Looks good to me Joe!

best regards,

-- daniel

On 11/10/16 21:29, Joe Wang wrote:

Hi,

Please review a fix for a NPE when the source contains empty namespace.

JBS: https://bugs.openjdk.java.net/browse/JDK-8152530
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/

Thanks,
Joe

RFR (JAXP) 8152530: NullPointerException when xmlns=""

[Joe Wang]

Hi,

Please review a fix for a NPE when the source contains empty namespace.

JBS: https://bugs.openjdk.java.net/browse/JDK-8152530
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/

Thanks,
Joe

Re: using jshell in executable UNIX scripts

[Paul Sandoz]

Hi Peter,

I was wondering when this would be discussed :-)

This potentially starts moving down the slippery slope of what it means to be 
Java syntax and what it means to execute. Larger topics to be thought about.

Here is some interesting discussion about Go:

  http://golangcookbook.com/chapters/running/shebang/

Surprisingly you can make it work in bash (from the above link):

  //$JDK_HOME/bin/jshell $0 $@; exit $?
  System.out.println("HELLO");

  /exit

Paul.

> On 10 Oct 2016, at 04:03, Peter Levart  wrote:
> 
> Hi,
> 
> I don't know if this is the right list to discuss this, so please direct me 
> to a more suitable place if there is one.
> 
> "jshell" command is a very nice interactive Java shell, but it could also be 
> used for scripting. An executable script in any major UNIX OS is a textual 
> file with executable permissions that starts with the following two 
> characters: #!
> The rest of the 1st line is the path to the interpreter executable and any 
> arguments passed to it. The last argument passed to the interpreter is the 
> path to the executable script. In case of jshell, one would want such script 
> to be written like:
> 
> #!/home/peter/Apps64/jdk9/bin/jshell
> 
> System.out.println("Hello World!");
> 
> /exit
> 
> 
> The problem is that jshell tries to parse the 1st line using jshell syntax 
> and the result of running above executable script is:
> 
> | Error:
> |  illegal character: '#'
> | #!/home/peter/Apps64/jdk9/bin/jshell
> |  ^
> |  Error:
> |  illegal start of expression
> |  #!/home/peter/Apps64/jdk9/bin/jshell
> |^
> Hello World!
> 
> 
> The script is actually executed, but the syntax error encountered in the 1st 
> line is printed too.
> 
> Would it be possible for jshell to skip 1st line if it starts with characters 
> #! like other shells do?
> 
> 
> Regards, Peter
> 

Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

[Lance Andersen]

The changes look good Mandy

> On Oct 11, 2016, at 2:03 PM, Mandy Chung  wrote:
> 
> This patch updates jar, jlink, jmod tool to be a provider of the new tool 
> SPI.  Some tests are also updated to replace the use of internal APIs with 
> ToolProvider::findFirst to look up a tool provider.  There are more tests 
> that can be updated and something to be cleaned up in the future.
> 
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html
> 
> thanks
> Mandy

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

[Mandy Chung]

This patch updates jar, jlink, jmod tool to be a provider of the new tool SPI.  
Some tests are also updated to replace the use of internal APIs with 
ToolProvider::findFirst to look up a tool provider.  There are more tests that 
can be updated and something to be cleaned up in the future.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html

thanks
Mandy

Re: RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Peter Levart]

Hi Daniel.


On 10/11/2016 05:19 PM, Daniel Fuchs wrote:

Hi Peter,

I was looking at the test - I didn't see a case where the
same method would be declared by two unrelated interfaces
later implemented by the same class.
Do we have test cases to verify that:

public interface I1 {
public Object test(String blah);
}
public interface I2 {
public Object test(String blah);
}
public abstract class A implements I2 {}
public abstract class A1 extends A implements I1, I2 {}
public abstract class A2 implements I1, I2 {}
public abstract class A3 implements I2, I1 {}

A1.getMethods(), A2.getMethods(), A3.getMethods() return
the expected result?
In particular what should A2.class.getMethods()
and A3.class.getMethods() return?


import java.lang.reflect.Method;

public class Test {

static String toMethodsString(Class clazz) {
StringBuilder sb = new StringBuilder();
sb.append(clazz.getName()).append(": [");
boolean first = true;
for (Method m : clazz.getMethods()) {
if (m.getDeclaringClass() != Object.class) {
if (first)
first = false;
else
sb.append(", ");
sb.append(m);
}
}
sb.append("]");
return sb.toString();
}

public static void main(String[] args) throws Exception {
for (Class clazz : new Class[]{A1.class, A2.class, A3.class}) {
System.out.println(toMethodsString(clazz));
}
}
}

interface I1 {
Object test(String blah);
}

interface I2 {
Object test(String blah);
}

abstract class A implements I2 {}

abstract class A1 extends A implements I1, I2 {}

abstract class A2 implements I1, I2 {}

abstract class A3 implements I2, I1 {}



Above program, ignoring Object methods, executed with JDK 7, 8 and 
9+patch, returns the same results:


[peter@cube jdk9-dev-test]$ ~/Apps64/jdk1.7.0/bin/java -cp out Test

A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]


[peter@cube jdk9-dev-test]$ ~/Apps64/jdk1.8.0/bin/java -cp out Test

A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]


[peter@cube jdk9-dev-test]$ 
~/work/hg/jdk9-dev/build/linux-x86_64-normal-server-release/images/jdk/bin/java 
-cp out Test


A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]


...which is expected.

In above example, two abstract methods with same signature are inherited 
from two unrelated interfaces, but they are not later implemented by a 
class. If it was implemented (or overridden by an abstract class 
method), only the class method would show.


I agree that the test is not very systematic in covering all possible 
combinations. Maybe it is time to write new test. Let me see...


Regards, Peter



best regards,

-- daniel

On 10/10/16 09:04, Peter Levart wrote:

Just a note that this is still ready to be reviewed.

I was also told that JCK tests pass with the patch applied.

Regards, Peter

On 10/04/2016 03:40 PM, Peter Levart wrote:

Hi,

I have a fix for conformance (P2) bug (8062389
) that also fixes a
conformance (P3) bug (8029459
) and a performance
issue (8061950 ):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/ 




As Daniel Smith writes in 8029459
, the following
situation is as expected:

interface I { void m(); }
interface J extends I { void m(); }
interface K extends J {}
K.class.getMethods() = [ J.m ]

But the following has a different result although it should probably
have the same:

interface I { void m(); }
interface J extends I { void m(); }
interface K extends I, J {}
K.class.getMethods() = [ I.m, J.m ]

He then suggests the following algorithm:

An implementation of getMethods consistent with JLS 8 would include
the following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
1) The class's/interface's declared 

Re: RFR 8165793 : Provide an API to query if a ClassLoader is parallel capable

[Mandy Chung]

> On Oct 10, 2016, at 11:03 PM, Peter Levart  wrote:
> 
> Hi Brent,
> 
> 
> On 10/11/2016 12:44 AM, Brent Christian wrote:
>> On 10/10/16 2:30 PM, Mandy Chung wrote:
>>> 
>>> The patch looks fine.  It would be good to add @see
>>> #registerAsParallelCapable in this new method.  Also the first
>>> “parallel capable” occurrance in the class spec and the
>>> registerAsParallelCapable method spec to @linkplain
>>> #isParallelCapable.
>> 
>> Thanks for having a look, and for the suggestions.  I also added an @see 
>> #isParallelCapable in registerAsParallelCapable().
>> 
>> Webrev updated in place:
>> http://cr.openjdk.java.net/~bchristi/8165793/webrev.00/
>> 
>> -Brent
> 
> It would be marginally more efficient to check for parallelLockMap != null 
> instead of looking up the CL implementation class in the WeakHashMap within a 
> synchronized block, but I guess the performance of this method is not very 
> important, is it?

parallelLockMap != null is another way to check that.  Sinc this method isn’t 
performance critical, calling ParallelLoaders::isRegistered is easier to 
understand.

Mandy

Re: RFR 8165793 : Provide an API to query if a ClassLoader is parallel capable

[Alan Bateman]

On 10/10/2016 23:44, Brent Christian wrote:


On 10/10/16 2:30 PM, Mandy Chung wrote:


The patch looks fine.  It would be good to add @see
#registerAsParallelCapable in this new method.  Also the first
“parallel capable” occurrance in the class spec and the
registerAsParallelCapable method spec to @linkplain
#isParallelCapable.


Thanks for having a look, and for the suggestions.  I also added an 
@see #isParallelCapable in registerAsParallelCapable().


Webrev updated in place:
http://cr.openjdk.java.net/~bchristi/8165793/webrev.00/
The javadoc looks okay to me. One thing that would be good is to beef up 
the test to cover more scenarios, esp. loader L1 extends loader L2 where 
you've got 4 combination of capable/non-capable to test.


-Alan

Re: RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Daniel Fuchs]

Hi Peter,

I was looking at the test - I didn't see a case where the
same method would be declared by two unrelated interfaces
later implemented by the same class.
Do we have test cases to verify that:

public interface I1 {
public Object test(String blah);
}
public interface I2 {
public Object test(String blah);
}
public abstract class A implements I2 {}
public abstract class A1 extends A implements I1, I2 {}
public abstract class A2 implements I1, I2 {}
public abstract class A3 implements I2, I1 {}

A1.getMethods(), A2.getMethods(), A3.getMethods() return
the expected result?
In particular what should A2.class.getMethods()
and A3.class.getMethods() return?

best regards,

-- daniel

On 10/10/16 09:04, Peter Levart wrote:

Just a note that this is still ready to be reviewed.

I was also told that JCK tests pass with the patch applied.

Regards, Peter

On 10/04/2016 03:40 PM, Peter Levart wrote:

Hi,

I have a fix for conformance (P2) bug (8062389
) that also fixes a
conformance (P3) bug (8029459
) and a performance
issue (8061950 ):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/


As Daniel Smith writes in 8029459
, the following
situation is as expected:

interface I { void m(); }
interface J extends I { void m(); }
interface K extends J {}
K.class.getMethods() = [ J.m ]

But the following has a different result although it should probably
have the same:

interface I { void m(); }
interface J extends I { void m(); }
interface K extends I, J {}
K.class.getMethods() = [ I.m, J.m ]

He then suggests the following algorithm:

An implementation of getMethods consistent with JLS 8 would include
the following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
1) The class's/interface's declared (public) methods
2) The getMethods() of the superclass (if this is a class), minus any
that have a match in (1)
3) The getMethods() of each superinterface, minus any that have a
match in (1) or a _concrete_ match in (2) or a match from a
more-specific class/interface in (2) or (3)

But even that is not sufficient for the following situation:

interface E { void m(); }
interface F extends E { void m(); }
abstract class G implements E {}
abstract class H extends G implements F {}
H.class.getMethods() = [ E.m, F.m ]

The desired result of H.class.getMethods() = [ F.m ]

So a more precise definition is required which is implemented in the fix.

The getMethods() implementation partitions the union of the following
methods:

1) The class's/interface's declared public methods
2) The getMethods() of the superclass (if this is a class)
3) The non-static getMethods() of each direct superinterface

...into equivalence classes (sets) of methods with same signature
(return type, name, parameter types). Within each such set, only the
"most specific" methods are kept and others are discarded. The union
of the kept methods is the result.

The "more specific" is defined as a partial order within a set of
methods of same signature:

Method A is more specific than method B if:
- A is declared by a class and B is declared by an interface; or
- A is declared by the same type as or a subtype of B's declaring type
and both are either declared by classes or both by interfaces (clearly
if A and B are declared by the same type and have the same signature,
they are the same method)

If A and B are distinct elements from the set of methods with same
signature, then either:
- A is more specific than B; or
- B is more specific than A; or
- A and B are incomparable

A sub-set of "most specific" methods are the methods from the set
where for each such method M, there is no method N != M such that N is
"more specific" than M.

There can be more than one "most specific" method for a particular
signature as they can be inherited from multiple unrelated interfaces,
but:
- javac prevents compilation when among multiply inherited methods
with same signature there is at least one default method, so in
practice, getMethods() will only return multiple methods with same
signature if they are abstract interface methods. With one exception:
bridge methods that are created by javac for co-variant overrides are
default methods in interfaces. For example:

interface I { Object m(); }
interface J1 extends I { Map m(); }
interface J2 extends I { HashMap m(); }
interface K extends J1, J2 {}

K.class.getMethods() = [ default Object j1.m, abstract Map j1.m,
default Object j2.m, abstract HashMap j2.m ]

But this is an extreme case where one can still expect multiple
non-abstract methods with same signature, but different declaring
type, returned from getMethods().

In order to also fix 8062389
, getMethod() and
getMethods() share the same consolidation logic that results in a set

Re: RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Alan Bateman]

On 10/10/2016 09:04, Peter Levart wrote:

Just a note that this is still ready to be reviewed.

I was also told that JCK tests pass with the patch applied.
I've taken a first pass over this and it looks quite good. I do intend 
to do a second pass and I hope others will help review this. One things 
we'll probably need to do is write a release note so that the behavioral 
change is documented, I'm trying to think if there are scenarios where 
the historical inconsistency could cause something to work that won't 
work now.


-Alan