Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Sundararajan Athijegannathan]

Looks good

-Sundar

On 14/03/18, 5:49 AM, David Lloyd wrote:

On Tue, Mar 13, 2018 at 7:16 PM, David Lloyd  wrote:

On Tue, Mar 13, 2018 at 6:31 PM, mandy chung  wrote:

I prefer to keep the original test case i.e. create a proxy class from
Runnable and Observer.   Instead add a new test case to create a proxy class
with ClashWithRunnable, Runnable and Observer and verify that it does not
include static methods.  I would add another static method
ClashWithRunnable::foo (no name clash) and verify
proxyClass::getDeclaredMethods does not contain foo.

OK, done.  It's a little bigger now so I'm attaching it.

Apologies, my IDE put it in the wrong directory.  Here's a better one.
I ran it by hand as jtreg and I are not presently on speaking terms.

Re: Raw String Literal Library Support

[Xueming Shen]

On 3/13/18, 5:12 PM, Jonathan Bluett-Duncan wrote:

Paul,

AFAICT, one sort of behaviour which String.split() allows which
Pattern.splitAsStream() and the proposed String.splits() don't is allowing
a negative limit, e.g. String.split(string, -1).

Over at http://errorprone.info/bugpattern/StringSplitter, they argue that a
limit of -1 has less surprising behaviour than the default of 0, because
e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
produces [""] (array with an empty string), which IMO is not consistent.

This compares with ":".split(":", -1) and "".split(":", -1) which produce
["", ""] (array with two empty strings, each representing ends of `:`) and
[] (empty array) respectively - more consistent IMO.

Should String.splits(`\n|\r\n?`) follow the behaviour of String.split(...,
0) or String.split(..., -1)?  I'd personally argue for the latter.


While these look really confusing, but ":".split(":", n) and 
"".split(":", n) are really two
different scenario. One is for a matched delimiter and the other is a 
split with no
matched delimiter, in which the spec specifies clearly that it returns 
the original string,
in this case the empty string "". Arguably these two don't have to be 
"consistent".


Personally I think the returned list/array from string.split(regex, -1) 
might be kinda of
"surprising" for end user, in which it has a "trailing" empty string, as 
it appears to be
useless in most use scenario and you probably have to do some special 
deal with it.


-Sherman





Cheers,
Jonathan

On 13 March 2018 at 23:22, Paul Sandoz  wrote:




On Mar 13, 2018, at 3:49 PM, John Rose  wrote:

On Mar 13, 2018, at 6:47 AM, Jim Laskey  wrote:

…
A. Line support.

public Stream  lines()


Suggest factoring this as:

public Stream  splits(String regex) { }

+1

This is a natural companion to the existing array returning method (as it
was the case on Pattern when we added splitAsStream), where one can use a
limit() operation to achieve the same effect as the limit parameter on the
array returning method.



public Stream  lines() { return splits(`\n|\r\n?`); }


See also Files/BufferedReader.lines. (Without going into details
Files.lines has some interesting optimizations.)

Paul.

Re: Raw String Literal Library Support

[Jonathan Bluett-Duncan]

Sorry, I should really run things in an IDE before posting code examples
and results!

For examples ":".split(":", 0) and "".split(":", 0), they actually produce
[] and [""] respectively (which I still argue is inconsistent and undesired
for the proposed String.splits()).

For examples ":".split(":", -1) and "".split(":", -1), they actually
produce ["", ""] and [""] respectively, which I like better.

Cheers,
Jonathan

On 14 March 2018 at 00:12, Jonathan Bluett-Duncan 
wrote:

> Paul,
>
> AFAICT, one sort of behaviour which String.split() allows which
> Pattern.splitAsStream() and the proposed String.splits() don't is allowing
> a negative limit, e.g. String.split(string, -1).
>
> Over at http://errorprone.info/bugpattern/StringSplitter, they argue that
> a limit of -1 has less surprising behaviour than the default of 0, because
> e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
> produces [""] (array with an empty string), which IMO is not consistent.
>
> This compares with ":".split(":", -1) and "".split(":", -1) which produce
> ["", ""] (array with two empty strings, each representing ends of `:`) and
> [] (empty array) respectively - more consistent IMO.
>
> Should String.splits(`\n|\r\n?`) follow the behaviour of String.split(...,
> 0) or String.split(..., -1)?  I'd personally argue for the latter.
>
> Cheers,
> Jonathan
>
> On 13 March 2018 at 23:22, Paul Sandoz  wrote:
>
>>
>>
>> > On Mar 13, 2018, at 3:49 PM, John Rose  wrote:
>> >
>> > On Mar 13, 2018, at 6:47 AM, Jim Laskey 
>> wrote:
>> >>
>> >> …
>> >> A. Line support.
>> >>
>> >> public Stream lines()
>> >>
>> >
>> > Suggest factoring this as:
>> >
>> > public Stream splits(String regex) { }
>>
>> +1
>>
>> This is a natural companion to the existing array returning method (as it
>> was the case on Pattern when we added splitAsStream), where one can use a
>> limit() operation to achieve the same effect as the limit parameter on the
>> array returning method.
>>
>>
>> > public Stream lines() { return splits(`\n|\r\n?`); }
>> >
>>
>> See also Files/BufferedReader.lines. (Without going into details
>> Files.lines has some interesting optimizations.)
>>
>> Paul.
>
>
>

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[David Lloyd]

On Tue, Mar 13, 2018 at 7:16 PM, David Lloyd  wrote:
> On Tue, Mar 13, 2018 at 6:31 PM, mandy chung  wrote:
>> I prefer to keep the original test case i.e. create a proxy class from
>> Runnable and Observer.   Instead add a new test case to create a proxy class
>> with ClashWithRunnable, Runnable and Observer and verify that it does not
>> include static methods.  I would add another static method
>> ClashWithRunnable::foo (no name clash) and verify
>> proxyClass::getDeclaredMethods does not contain foo.
>
> OK, done.  It's a little bigger now so I'm attaching it.

Apologies, my IDE put it in the wrong directory.  Here's a better one.
I ran it by hand as jtreg and I are not presently on speaking terms.
-- 
- DML
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java 
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
  */
 for (Class intf : interfaces) {
 for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
 }
 }
 
diff --git a/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java 
b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
new file mode 100644
index 000..5ad752880b3
--- /dev/null
+++ b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8188240
+ * @summary This is a test to ensure that proxies do not inherit static 
methods.
+ *
+ * @build ProxyClashTest
+ * @run main ProxyClashTest
+ */
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.Observer;
+
+public class ProxyClashTest {
+
+public interface ClashWithRunnable {
+static int run() { return 123; }
+
+static void foo() {}
+}
+
+public static void main(String[] args) {
+
+System.err.println(
+"\nDynamic proxy API static method clash test\n");
+
+try {
+Class[] interfaces =
+new Class[] { ClashWithRunnable.class, Runnable.class, 
Observer.class };
+
+ClassLoader loader = ClassLoader.getSystemClassLoader();
+
+/*
+ * Generate a proxy class.
+ */
+Class proxyClass = Proxy.getProxyClass(loader, interfaces);
+System.err.println("+ generated proxy class: " + proxyClass);
+
+for (Method method : proxyClass.getDeclaredMethods()) {
+if (method.getName().equals("run") && method.getReturnType() 
== int.class) {
+throw new RuntimeException("proxy inherited a static 
method");
+}
+if (method.getName().equals("foo")) {
+throw new RuntimeException("proxy inherited a static 
method");
+}
+}
+
+System.err.println("\nTEST PASSED");
+
+} catch (Throwable e) {
+System.err.println("\nTEST FAILED:");
+e.printStackTrace();
+throw new RuntimeException("TEST FAILED: " + e.toString());
+}
+}
+}

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[David Lloyd]

On Tue, Mar 13, 2018 at 6:31 PM, mandy chung  wrote:
> I prefer to keep the original test case i.e. create a proxy class from
> Runnable and Observer.   Instead add a new test case to create a proxy class
> with ClashWithRunnable, Runnable and Observer and verify that it does not
> include static methods.  I would add another static method
> ClashWithRunnable::foo (no name clash) and verify
> proxyClass::getDeclaredMethods does not contain foo.

OK, done.  It's a little bigger now so I'm attaching it.


-- 
- DML
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java 
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
  */
 for (Class intf : interfaces) {
 for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
 }
 }
 
diff --git a/test/jdk/java/lang/reflect/Proxy/src/test/ProxyClashTest.java 
b/test/jdk/java/lang/reflect/Proxy/src/test/ProxyClashTest.java
new file mode 100644
index 000..5ad752880b3
--- /dev/null
+++ b/test/jdk/java/lang/reflect/Proxy/src/test/ProxyClashTest.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8188240
+ * @summary This is a test to ensure that proxies do not inherit static 
methods.
+ *
+ * @build ProxyClashTest
+ * @run main ProxyClashTest
+ */
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.Observer;
+
+public class ProxyClashTest {
+
+public interface ClashWithRunnable {
+static int run() { return 123; }
+
+static void foo() {}
+}
+
+public static void main(String[] args) {
+
+System.err.println(
+"\nDynamic proxy API static method clash test\n");
+
+try {
+Class[] interfaces =
+new Class[] { ClashWithRunnable.class, Runnable.class, 
Observer.class };
+
+ClassLoader loader = ClassLoader.getSystemClassLoader();
+
+/*
+ * Generate a proxy class.
+ */
+Class proxyClass = Proxy.getProxyClass(loader, interfaces);
+System.err.println("+ generated proxy class: " + proxyClass);
+
+for (Method method : proxyClass.getDeclaredMethods()) {
+if (method.getName().equals("run") && method.getReturnType() 
== int.class) {
+throw new RuntimeException("proxy inherited a static 
method");
+}
+if (method.getName().equals("foo")) {
+throw new RuntimeException("proxy inherited a static 
method");
+}
+}
+
+System.err.println("\nTEST PASSED");
+
+} catch (Throwable e) {
+System.err.println("\nTEST FAILED:");
+e.printStackTrace();
+throw new RuntimeException("TEST FAILED: " + e.toString());
+}
+}
+}

Re: Raw String Literal Library Support

[Jonathan Bluett-Duncan]

Paul,

AFAICT, one sort of behaviour which String.split() allows which
Pattern.splitAsStream() and the proposed String.splits() don't is allowing
a negative limit, e.g. String.split(string, -1).

Over at http://errorprone.info/bugpattern/StringSplitter, they argue that a
limit of -1 has less surprising behaviour than the default of 0, because
e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
produces [""] (array with an empty string), which IMO is not consistent.

This compares with ":".split(":", -1) and "".split(":", -1) which produce
["", ""] (array with two empty strings, each representing ends of `:`) and
[] (empty array) respectively - more consistent IMO.

Should String.splits(`\n|\r\n?`) follow the behaviour of String.split(...,
0) or String.split(..., -1)?  I'd personally argue for the latter.

Cheers,
Jonathan

On 13 March 2018 at 23:22, Paul Sandoz  wrote:

>
>
> > On Mar 13, 2018, at 3:49 PM, John Rose  wrote:
> >
> > On Mar 13, 2018, at 6:47 AM, Jim Laskey  wrote:
> >>
> >> …
> >> A. Line support.
> >>
> >> public Stream lines()
> >>
> >
> > Suggest factoring this as:
> >
> > public Stream splits(String regex) { }
>
> +1
>
> This is a natural companion to the existing array returning method (as it
> was the case on Pattern when we added splitAsStream), where one can use a
> limit() operation to achieve the same effect as the limit parameter on the
> array returning method.
>
>
> > public Stream lines() { return splits(`\n|\r\n?`); }
> >
>
> See also Files/BufferedReader.lines. (Without going into details
> Files.lines has some interesting optimizations.)
>
> Paul.

Re: RFR 8189230: JDK method:java.lang.Integer.numberOfLeadingZeros(int) can be optimized

[Brian Burkhalter]

On Mar 13, 2018, at 4:42 PM, Paul Sandoz  wrote:

> Given how trivial the changes are it looks ok, but not sure it really matters 
> in practice.

Nor do I.

> 
> —
> 
> Note that it’s possible to selectively disable intrinsics. For future 
> reference, when you want to avoid copying code if you happen to benchmark 
> intrinsics in the future:
> 
>  -XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_numberOfLeadingZeros_i

Yes, I knew about that.

> But you have to know the intrinsic name, you can find ‘em here:
> 
>  
> http://hg.openjdk.java.net/jdk/jdk/file/63eceefeb347/src/hotspot/share/classfile/vmSymbols.hpp#l805

This latter is very helpful, thanks.

Brian

Re: RFR 8189230: JDK method:java.lang.Integer.numberOfLeadingZeros(int) can be optimized

[Paul Sandoz]

Given how trivial the changes are it looks ok, but not sure it really matters 
in practice.

—

Note that it’s possible to selectively disable intrinsics. For future 
reference, when you want to avoid copying code if you happen to benchmark 
intrinsics in the future:

  -XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_numberOfLeadingZeros_i

But you have to know the intrinsic name, you can find ‘em here:

  
http://hg.openjdk.java.net/jdk/jdk/file/63eceefeb347/src/hotspot/share/classfile/vmSymbols.hpp#l805

Paul.

> On Mar 13, 2018, at 4:14 PM, Brian Burkhalter  
> wrote:
> 
> https://bugs.openjdk.java.net/browse/JDK-8189230
> 
> The change included below improves the performance of 
> {Integer,Long}.numberOfLeadingZeros primarily for negative parameters by 20% 
> to 33% as measured by JMH benchmarks. For details please refer to the bug 
> report. Although on certain platforms there could be an intrinsic for the 
> methods in question, given the simplicity of the change it seems worth making.
> 
> Thanks,
> 
> Brian
> 
> --- a/src/java.base/share/classes/java/lang/Integer.java
> +++ b/src/java.base/share/classes/java/lang/Integer.java
> @@ -1625,8 +1625,8 @@
> @HotSpotIntrinsicCandidate
> public static int numberOfLeadingZeros(int i) {
> // HD, Figure 5-6
> -if (i == 0)
> -return 32;
> +if (i <= 0)
> +return i == 0 ? 32 : 0;
> int n = 1;
> if (i >>> 16 == 0) { n += 16; i <<= 16; }
> if (i >>> 24 == 0) { n +=  8; i <<=  8; }
> 
> --- a/src/java.base/share/classes/java/lang/Long.java
> +++ b/src/java.base/share/classes/java/lang/Long.java
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1994, 2018, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -1771,8 +1771,8 @@
> @HotSpotIntrinsicCandidate
> public static int numberOfLeadingZeros(long i) {
> // HD, Figure 5-6
> - if (i == 0)
> -return 64;
> + if (i <= 0)
> +return i == 0 ? 64 : 0;
> int n = 1;
> int x = (int)(i >>> 32);
> if (x == 0) { n += 32; x = (int)i; }
> 

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

On 3/13/18 4:06 PM, David Lloyd wrote:

I worked up a little patch for 8188240.  I was able to co-opt an
existing test which now fails before the patch and passes after.  It's
a tiny patch so I'm including it inline.  I've CC'd Mandy because she
filed the original bug.

Here's the patch (use patch -p1 to apply):

 cut --- 8< --- cut 
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
   */
  for (Class intf : interfaces) {
  for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
  }
  }


This looks okay.



diff --git a/test/jdk/java/lang/reflect/Proxy/Basic1.java
b/test/jdk/java/lang/reflect/Proxy/Basic1.java
index b2440ccd9f9..4a21470b1be 100644
--- a/test/jdk/java/lang/reflect/Proxy/Basic1.java
+++ b/test/jdk/java/lang/reflect/Proxy/Basic1.java
@@ -36,6 +36,10 @@ import java.util.*;

  public class Basic1 {

+public interface ClashWithRunnable {
+static int run() { return 123; }
+}
+
  public static void main(String[] args) {

  System.err.println(
@@ -43,7 +47,7 @@ public class Basic1 {

  try {
  Class[] interfaces =
-new Class[] { Runnable.class, Observer.class };
+new Class[] { ClashWithRunnable.class,
Runnable.class, Observer.class };

  ClassLoader loader = ClassLoader.getSystemClassLoader();

 cut --- 8< --- cut 


I prefer to keep the original test case i.e. create a proxy class from 
Runnable and Observer.   Instead add a new test case to create a proxy 
class with ClashWithRunnable, Runnable and Observer and verify that it 
does not include static methods.  I would add another static method 
ClashWithRunnable::foo (no name clash) and verify 
proxyClass::getDeclaredMethods does not contain foo.


Mandy

Re: Raw String Literal Library Support

[Paul Sandoz]

> On Mar 13, 2018, at 3:49 PM, John Rose  wrote:
> 
> On Mar 13, 2018, at 6:47 AM, Jim Laskey  wrote:
>> 
>> …
>> A. Line support.
>> 
>> public Stream lines()
>> 
> 
> Suggest factoring this as:
> 
> public Stream splits(String regex) { }

+1

This is a natural companion to the existing array returning method (as it was 
the case on Pattern when we added splitAsStream), where one can use a limit() 
operation to achieve the same effect as the limit parameter on the array 
returning method.


> public Stream lines() { return splits(`\n|\r\n?`); }
> 

See also Files/BufferedReader.lines. (Without going into details Files.lines 
has some interesting optimizations.)

Paul.

RFR 8189230: JDK method:java.lang.Integer.numberOfLeadingZeros(int) can be optimized

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8189230

The change included below improves the performance of 
{Integer,Long}.numberOfLeadingZeros primarily for negative parameters by 20% to 
33% as measured by JMH benchmarks. For details please refer to the bug report. 
Although on certain platforms there could be an intrinsic for the methods in 
question, given the simplicity of the change it seems worth making.

Thanks,

Brian

--- a/src/java.base/share/classes/java/lang/Integer.java
+++ b/src/java.base/share/classes/java/lang/Integer.java
@@ -1625,8 +1625,8 @@
 @HotSpotIntrinsicCandidate
 public static int numberOfLeadingZeros(int i) {
 // HD, Figure 5-6
-if (i == 0)
-return 32;
+if (i <= 0)
+return i == 0 ? 32 : 0;
 int n = 1;
 if (i >>> 16 == 0) { n += 16; i <<= 16; }
 if (i >>> 24 == 0) { n +=  8; i <<=  8; }

--- a/src/java.base/share/classes/java/lang/Long.java
+++ b/src/java.base/share/classes/java/lang/Long.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1771,8 +1771,8 @@
 @HotSpotIntrinsicCandidate
 public static int numberOfLeadingZeros(long i) {
 // HD, Figure 5-6
- if (i == 0)
-return 64;
+ if (i <= 0)
+return i == 0 ? 64 : 0;
 int n = 1;
 int x = (int)(i >>> 32);
 if (x == 0) { n += 32; x = (int)i; }

[PATCH] 8188240: Reflection Proxy should skip static methods

[David Lloyd]

I worked up a little patch for 8188240.  I was able to co-opt an
existing test which now fails before the patch and passes after.  It's
a tiny patch so I'm including it inline.  I've CC'd Mandy because she
filed the original bug.

Here's the patch (use patch -p1 to apply):

 cut --- 8< --- cut 
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
  */
 for (Class intf : interfaces) {
 for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
 }
 }

diff --git a/test/jdk/java/lang/reflect/Proxy/Basic1.java
b/test/jdk/java/lang/reflect/Proxy/Basic1.java
index b2440ccd9f9..4a21470b1be 100644
--- a/test/jdk/java/lang/reflect/Proxy/Basic1.java
+++ b/test/jdk/java/lang/reflect/Proxy/Basic1.java
@@ -36,6 +36,10 @@ import java.util.*;

 public class Basic1 {

+public interface ClashWithRunnable {
+static int run() { return 123; }
+}
+
 public static void main(String[] args) {

 System.err.println(
@@ -43,7 +47,7 @@ public class Basic1 {

 try {
 Class[] interfaces =
-new Class[] { Runnable.class, Observer.class };
+new Class[] { ClashWithRunnable.class,
Runnable.class, Observer.class };

 ClassLoader loader = ClassLoader.getSystemClassLoader();

 cut --- 8< --- cut 

-- 
- DML

Re: Raw String Literal Library Support

[John Rose]

On Mar 13, 2018, at 6:47 AM, Jim Laskey  wrote:
> 
> …
> A. Line support.
> 
> public Stream lines()
> 

Suggest factoring this as:

 public Stream splits(String regex) { }
 public Stream lines() { return splits(`\n|\r\n?`); }

The reason is that "splits" is useful with several other patterns.
For raw strings, splits(`\n`) is a more efficient way to get the same
result (because they normalize CR NL? to NL).  There's also a
nifty unicode-oriented pattern splits(`\R`) which matches a larger
set of line terminations.  And of course splits(":") or splits(`\s`) will
be old friends.  A new friend might be paragraph splitting splits(`\n\n`).

Splitting is old, as Remi points out, but new thing is supplying the
stream-style fluent notation starting from a (potentially) large string
constant.

> B. Additions to basic trim methods. In addition to margin methods trimIndent 
> and trimMarkers described below in Section C, it would be worth introducing 
> trimLeft and trimRight to augment the longstanding trim method. A key 
> question is how trimLeft and trimRight should detect whitespace, because 
> different definitions of whitespace exist in the library. 
> ...
> That sets up several kinds of whitespace; trim's whitespace (TWS), Character 
> whitespace (CWS) and the union of the two (UWS). TWS is a fast test. CWS is a 
> slow test. UWS is fast for Latin1 and slow-ish for UTF-16. 

For the record, even though we are not talking performance much,
CWS is not significantly slower than UWS.  You can use a 64-bit int
constant for a bitmask and check for an arbitrary subset of the first
64 ASCII code points in one or two machine instructions.

> We are recommending that trimLeft and trimRight use UWS, leave trim alone to 
> avoid breaking the world and then possibly introduce trimWhitespace that uses 
> UWS.

Putting aside the performance question, I have to ask if compatibility
with TWS is at all important.  (Don't know the answer, suspect not.)
> …
> C. Margin management. With introduction of multi-line Raw String Literals, 
> developers will have to deal with the extraneous spacing introduced by 
> indenting and formatting string bodies. 
> 
> Note that for all the methods in this group, if the first line is empty then 
> it is removed and if the last is empty then it is removed. This removal 
> provides a means for developers that use delimiters on separate lines to 
> bracket string bodies. Also note, that all line separators are replaced with 
> \n.

(As a bonus, margin management gives a story for escaping leading and trailing
backticks.  If your string is a single line, surround it with pipe characters 
`|asdf|`.
If your string is multiple lines, surround it with blank lines easy to do.  
Either
pipes or newlines will protect backticks from merging into quotes.)

There's a sort of beauty contest going on here between indents and
markers.  I often prefer markers, but I see how indents will often win
the contest.  I'll pre-emptively disagree with anyone who observes
that we only need one of the two.

> public String trimMarkers(String leftMarker, String rightMarker)

I like this function and anticipate using it.  (I use similar things in
shell script here-files.)  Thanks for including end-of-line markers
in the mix.  This allows lines with significant *trailing* whitespace
to protect that whitespace as well as *leading* whitespace.

Suggestion:  Give users a gentle nudge toward the pipe character by
making it a default argument so trimMarkers() => trimMarkers("|","|").

Suggestion:  Allow the markers to be regular expressions.
(So `\|` would be the default.)

> 
> D. Escape management. Since Raw String Literals do not interpret Unicode 
> escapes (\u) or escape sequences (\n, \b, etc), we need to provide a 
> scheme for developers who just want multi-line strings but still have escape 
> sequences interpreted.

This all looks good.

Thanks,

— John

Re: Raw String Literal Library Support

[John Rose]

On Mar 13, 2018, at 11:30 AM, Volker Simonis  wrote:
> 
> Would it make sense to have a versions of "lines(LINE_TERM lt)" which
> take a single, concrete form of line terminator?

(Regular expressions for the win!)

Re: Raw String Literal Library Support

[John Rose]

On Mar 13, 2018, at 11:42 AM, Remi Forax  wrote:
> 
> it already exists :)
>  Stream stream = Pattern.compile("\n|\r\n|\r").splitAsStream(string);

You want ` instead of " there!

Somebody added support recently for `\R`, which is a more unicode-flavored
version of your pattern (or just `\n`).  Last time I looked it was missing; 
kudos
to whoever added it in.

There should be a fluent streamy syntax for splitting a string, 
string.splits(pat).
Java's incomplete embrace of fluent syntax is old news, *but* there is something
new here:  String expression size.

The raw strings are much larger than classic strings, and so they seem to need 
some
notational assistance that doesn't always require them to be enclosed in round 
parens
and mixed with other arguments.  Having more fluent methods on String seems like
a good move here.

This goes beyond raw strings, and parsing is hard, but maybe there's room for
richer versions of String.lines or String.splits, which can deliver both the 
surrounding
whitespace and one or more fields, for each line (or each paragraph or 
whatever):

public Stream matchResults(String regex) {
  return Pattern.compile(regex).matcher(this).results();
}

The point is that a MatchResult delivers both the whole substring and any groups
embedded in it as part of the match.  Plus indexes, which is nice sometimes.

— John

Re: RFR 8198889 Clarify the throwing of exceptions from ConstantBootstraps.invoke

[mandy chung]

+1

Mandy

On 3/13/18 11:37 AM, Paul Sandoz wrote:

Hi,

Please review these minor tweaks to the specification of 
ConstantBootstraps.invoke to clarify the throwing of exceptions. This was the 
result of discussion with the JCK team.

A CSR has also been filed and requires a reviewer.

   https://bugs.openjdk.java.net/browse/JDK-8199540

Thanks,
Paul.

diff -r 7c795d301dbf 
src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
--- a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java  
Mon Mar 12 16:09:18 2018 -0700
+++ b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java  
Tue Mar 13 11:27:34 2018 -0700
@@ -104,10 +104,10 @@
   *
   * @param lookup the lookup context describing the class performing the
   * operation (normally stacked by the JVM)
+ * @param name the name of the constant to return, which must exactly match
+ * an enum constant in the specified type.
   * @param type the {@code Class} object describing the enum type for which
   * a constant is to be returned
- * @param name the name of the constant to return, which must exactly match
- * an enum constant in the specified type.
   * @param  The enum type for which a constant value is to be returned
   * @return the enum constant of the specified enum type with the
   * specified name
@@ -208,20 +208,25 @@
  /**
   * Returns the result of invoking a method handle with the provided
   * arguments.
+ * 
+ * This method behaves as if the method handle to be invoked is the result
+ * of adapting the given method handle, via {@link MethodHandle#asType}, to
+ * adjust the return type to the desired type.
   *
   * @param lookup unused
   * @param name unused
- * @param type the type of the value to be returned, which must be
+ * @param type the desired type of the value to be returned, which must be
   * compatible with the return type of the method handle
   * @param handle the method handle to be invoked
   * @param args the arguments to pass to the method handle, as if with
   * {@link MethodHandle#invokeWithArguments}.  Each argument may be
   * {@code null}.
   * @return the result of invoking the method handle
- * @throws WrongMethodTypeException if the handle's return type cannot be
- * adjusted to the desired type
- * @throws ClassCastException if an argument cannot be converted by
- * reference casting
+ * @throws WrongMethodTypeException if the handle's method type cannot be
+ * adjusted to take the given number of arguments, or if the handle's 
return
+ * type cannot be adjusted to the desired type
+ * @throws ClassCastException if an argument or the result produced by
+ * invoking the handle cannot be converted by reference casting
   * @throws Throwable anything thrown by the method handle invocation
   */
  public static Object invoke(MethodHandles.Lookup lookup, String name, 
Class type,

Re: Raw String Literal Library Support

[Mark Derricutt]

On 14 Mar 2018, at 7:42, Remi Forax wrote:

> it already exists :)
>   Stream stream = Pattern.compile("\n|\r\n|\r").splitAsStream(string);
>
> Rémi

One that worked, or had support for grey-space would be a nice addition when 
working with RSL tho, unless grey-space is automatically handled at the 
compiler level ( don't believe I saw that mentioned anywhere ).

Mark



---
"The ease with which a change can be implemented has no relevance at all to 
whether it is the right change for the (Java) Platform for all time."  
Mark Reinhold.

Mark Derricutt
http://www.theoryinpractice.net
http://www.chaliceofblood.net
http://plus.google.com/+MarkDerricutt
http://twitter.com/talios
http://facebook.com/mderricutt

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 13, 2018, at 12:24 PM, Alan Bateman  wrote:

> As relative paths are command then it might be simpler to just change the 
> static initializer

Static initializer or constructor?

> to initialize userDir to normalize(props.getProperty("user.dir")).

I was originally going to put that in the constructor but did not know about 
calling the normalize() instance method from the ctor.

Brian

Re: RFR: 8199471: Enable generation of callSiteForms at link time

[Paul Sandoz]

Invokers.java
—

Looks good.

Minor comment:

 664 /* Placeholder class for Invokers generated ahead of time */
 665 final class Holder {}
 666 
 667 /* Placeholder class for callSiteForms generated ahead of time */
 668 final class CSHolder {}

is it easy for you to change, for clarity, Holder to InvokersHolder and 
CSHolder to CallSiteHolder?

Thanks,
Paul.


> On Mar 12, 2018, at 11:59 AM, Claes Redestad  
> wrote:
> 
> Hi,
> 
> LFs generated via Invokers.callSiteForm only depend on the MethodTypeForm and 
> link type,
> and can be generated ahead of time by jlink --generate-jli-classes:
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8199471/open.00/
> Bug:https://bugs.openjdk.java.net/browse/JDK-8199471
> 
> The current training runs done during build shows this is somewhat effective 
> at reducing
> the number of LF classes we generate at runtime, avoiding spinning up up to 6 
> classes
> during bootstrap.
> 
> Thanks!
> 
> /Claes

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 12/03/2018 18:32, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8198997
http://cr.openjdk.java.net/~bpb/8198997/webrev.00/

Lazily cache the value of the user.dir property on first access. This change is 
for Windows only as there does not appear to be any percentage in doing 
something similar on Unix.

The user.dir property comes from GetCurrentDirectoryW so it will be 
normalized anyway. Although unsupported, I guess there may be some 
deployments setting this property on the command line so it is safer to 
normalize. As relative paths are command then it might be simpler to 
just change the static initializer to initialize userDir to 
normalize(props.getProperty("user.dir")).


-Alan

Re: RFR 8198889 Clarify the throwing of exceptions from ConstantBootstraps.invoke

[Lance Andersen]

Changes look reasonable Paul…

Best
Lance
> On Mar 13, 2018, at 2:37 PM, Paul Sandoz  wrote:
> 
> Hi,
> 
> Please review these minor tweaks to the specification of 
> ConstantBootstraps.invoke to clarify the throwing of exceptions. This was the 
> result of discussion with the JCK team.
> 
> A CSR has also been filed and requires a reviewer.
> 
>  https://bugs.openjdk.java.net/browse/JDK-8199540
> 
> Thanks,
> Paul.
> 
> diff -r 7c795d301dbf 
> src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
> --- a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
> Mon Mar 12 16:09:18 2018 -0700
> +++ b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
> Tue Mar 13 11:27:34 2018 -0700
> @@ -104,10 +104,10 @@
>  *
>  * @param lookup the lookup context describing the class performing the
>  * operation (normally stacked by the JVM)
> + * @param name the name of the constant to return, which must exactly 
> match
> + * an enum constant in the specified type.
>  * @param type the {@code Class} object describing the enum type for which
>  * a constant is to be returned
> - * @param name the name of the constant to return, which must exactly 
> match
> - * an enum constant in the specified type.
>  * @param  The enum type for which a constant value is to be returned
>  * @return the enum constant of the specified enum type with the
>  * specified name
> @@ -208,20 +208,25 @@
> /**
>  * Returns the result of invoking a method handle with the provided
>  * arguments.
> + * 
> + * This method behaves as if the method handle to be invoked is the 
> result
> + * of adapting the given method handle, via {@link MethodHandle#asType}, 
> to
> + * adjust the return type to the desired type.
>  *
>  * @param lookup unused
>  * @param name unused
> - * @param type the type of the value to be returned, which must be
> + * @param type the desired type of the value to be returned, which must 
> be
>  * compatible with the return type of the method handle
>  * @param handle the method handle to be invoked
>  * @param args the arguments to pass to the method handle, as if with
>  * {@link MethodHandle#invokeWithArguments}.  Each argument may be
>  * {@code null}.
>  * @return the result of invoking the method handle
> - * @throws WrongMethodTypeException if the handle's return type cannot be
> - * adjusted to the desired type
> - * @throws ClassCastException if an argument cannot be converted by
> - * reference casting
> + * @throws WrongMethodTypeException if the handle's method type cannot be
> + * adjusted to take the given number of arguments, or if the handle's 
> return
> + * type cannot be adjusted to the desired type
> + * @throws ClassCastException if an argument or the result produced by
> + * invoking the handle cannot be converted by reference casting
>  * @throws Throwable anything thrown by the method handle invocation
>  */
> public static Object invoke(MethodHandles.Lookup lookup, String name, 
> Class type,
> 

 
  

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

Re: Raw String Literal Library Support

[Remi Forax]

Hi Jim,

- Mail original -
> De: "Jim Laskey" 
> À: "core-libs-dev" 
> Envoyé: Mardi 13 Mars 2018 14:47:29
> Objet: Raw String Literal Library Support

> With the announcement of JEP 326 Raw String Literals, we would like to open 
> up a
> discussion with regards to RSL library support. Below are several implemented
> String methods that are believed to be appropriate. Please comment on those
> mentioned below including recommending alternate names or signatures.
> Additional methods can be considered if warranted, but as always, the bar for
> inclusion in String is high.
> 
> You should keep a couple things in mind when reviewing these methods.
> 
> Methods should be applicable to all strings, not just Raw String Literals.
> 
> The number of additional methods should be minimized, not adding every 
> possible
> method.
> 
> Don't put any emphasis on performance. That is a separate discussion.
> 
> Cheers,
> 
> -- Jim
> 
> A. Line support.
> 
> public Stream lines()
> Returns a stream of substrings extracted from this string partitioned by line
> terminators. Internally, the stream is implemented using a Spliteratorthat
> extracts one line at a time. The line terminators recognized are \n, \r\n and
> \r. This method provides versatility for the developer working with multi-line
> strings.

it already exists :)
  Stream stream = Pattern.compile("\n|\r\n|\r").splitAsStream(string);

Rémi

RFR 8198889 Clarify the throwing of exceptions from ConstantBootstraps.invoke

[Paul Sandoz]

Hi,

Please review these minor tweaks to the specification of 
ConstantBootstraps.invoke to clarify the throwing of exceptions. This was the 
result of discussion with the JCK team.

A CSR has also been filed and requires a reviewer.

  https://bugs.openjdk.java.net/browse/JDK-8199540

Thanks,
Paul.

diff -r 7c795d301dbf 
src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
--- a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java  
Mon Mar 12 16:09:18 2018 -0700
+++ b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java  
Tue Mar 13 11:27:34 2018 -0700
@@ -104,10 +104,10 @@
  *
  * @param lookup the lookup context describing the class performing the
  * operation (normally stacked by the JVM)
+ * @param name the name of the constant to return, which must exactly match
+ * an enum constant in the specified type.
  * @param type the {@code Class} object describing the enum type for which
  * a constant is to be returned
- * @param name the name of the constant to return, which must exactly match
- * an enum constant in the specified type.
  * @param  The enum type for which a constant value is to be returned
  * @return the enum constant of the specified enum type with the
  * specified name
@@ -208,20 +208,25 @@
 /**
  * Returns the result of invoking a method handle with the provided
  * arguments.
+ * 
+ * This method behaves as if the method handle to be invoked is the result
+ * of adapting the given method handle, via {@link MethodHandle#asType}, to
+ * adjust the return type to the desired type.
  *
  * @param lookup unused
  * @param name unused
- * @param type the type of the value to be returned, which must be
+ * @param type the desired type of the value to be returned, which must be
  * compatible with the return type of the method handle
  * @param handle the method handle to be invoked
  * @param args the arguments to pass to the method handle, as if with
  * {@link MethodHandle#invokeWithArguments}.  Each argument may be
  * {@code null}.
  * @return the result of invoking the method handle
- * @throws WrongMethodTypeException if the handle's return type cannot be
- * adjusted to the desired type
- * @throws ClassCastException if an argument cannot be converted by
- * reference casting
+ * @throws WrongMethodTypeException if the handle's method type cannot be
+ * adjusted to take the given number of arguments, or if the handle's 
return
+ * type cannot be adjusted to the desired type
+ * @throws ClassCastException if an argument or the result produced by
+ * invoking the handle cannot be converted by reference casting
  * @throws Throwable anything thrown by the method handle invocation
  */
 public static Object invoke(MethodHandles.Lookup lookup, String name, 
Class type,

Re: Raw String Literal Library Support

[Volker Simonis]

On Tue, Mar 13, 2018 at 2:47 PM, Jim Laskey  wrote:
> With the announcement of JEP 326 Raw String Literals, we would like to open 
> up a discussion with regards to RSL library support. Below are several 
> implemented String methods that are believed to be appropriate. Please 
> comment on those mentioned below including recommending alternate names or 
> signatures. Additional methods can be considered if warranted, but as always, 
> the bar for inclusion in String is high.
>
> You should keep a couple things in mind when reviewing these methods.
>
> Methods should be applicable to all strings, not just Raw String Literals.
>
> The number of additional methods should be minimized, not adding every 
> possible method.
>
> Don't put any emphasis on performance. That is a separate discussion.
>
> Cheers,
>
> -- Jim
>
> A. Line support.
>
> public Stream lines()
> Returns a stream of substrings extracted from this string partitioned by line 
> terminators. Internally, the stream is implemented using a Spliteratorthat 
> extracts one line at a time. The line terminators recognized are \n, \r\n and 
> \r. This method provides versatility for the developer working with 
> multi-line strings.

So "lines()" will support any mix of  "\n", "\r\n" and "\r" inside a
single string as line terminator?

Will "\n", "\r\n" and "\r" be parsed from left to right with one
character look-ahead? I.e.
\n = 1 newline
\n\r = 2 newlines (i.e. an empty line)
\n\r\n = 2 newlines (i.e. an empty line) because "\r\n" counts as a
single new line
\n\r\n\r = 3 newlines (i.e. two empty lines)

Would it make sense to have a versions of "lines(LINE_TERM lt)" which
take a single, concrete form of line terminator?

>  Example:
>
> String string = "abc\ndef\nghi";
> Stream stream = string.lines();
> List list = stream.collect(Collectors.toList());
>
>  Result:
>
>  [abc, def, ghi]
>
>
>  Example:
>
> String string = "abc\ndef\nghi";
> String[] array = string.lines().toArray(String[]::new);
>
>  Result:
>
>  [Ljava.lang.String;@33e5ccce // [abc, def, ghi]
>
>
>  Example:
>
> String string = "abc\ndef\r\nghi\rjkl";
> String platformString =
> string.lines().collect(joining(System.lineSeparator()));
>
>  Result:
>
>  abc
>  def
>  ghi
>  jkl
>
>
>  Example:
>
> String string = " abc  \n   def  \n ghi   ";
> String trimmedString =
>  string.lines().map(s -> s.trim()).collect(joining("\n"));
>
>  Result:
>
>  abc
>  def
>  ghi
>
>
>  Example:
>
> String table = `First Name  SurnamePhone
> Al  Albert 555-
> Bob Roberts555-
> Cal Calvin 555-
>`;
>
> // Extract headers
> String firstLine = table.lines().findFirst().orElse("");
> List headings = List.of(firstLine.trim().split(`\s{2,}`));
>
> // Build stream of maps
> Stream> stream =
> table.lines().skip(1)
>  .map(line -> line.trim())
>  .filter(line -> !line.isEmpty())
>  .map(line -> line.split(`\s{2,}`))
>  .map(columns -> {
>  List values = List.of(columns);
>  return IntStream.range(0, headings.size()).boxed()
>  .collect(toMap(headings::get, 
> values::get));
>  });
>
> // print all "First Name"
> stream.map(row -> row.get("First Name"))
>   .forEach(name -> System.out.println(name));
>
>  Result:
>
>  Al
>  Bob
>  Cal
> B. Additions to basic trim methods. In addition to margin methods trimIndent 
> and trimMarkers described below in Section C, it would be worth introducing 
> trimLeft and trimRight to augment the longstanding trim method. A key 
> question is how trimLeft and trimRight should detect whitespace, because 
> different definitions of whitespace exist in the library.
>
> trim itself uses the simple test less than or equal to the space character, a 
> fast test but not Unicode friendly.
>
> Character.isWhitespace(codepoint) returns true if codepoint one of the 
> following;
>
>SPACE_SEPARATOR.
>LINE_SEPARATOR.
>PARAGRAPH_SEPARATOR.
>'\t', U+0009 HORIZONTAL TABULATION.
>'\n', U+000A LINE FEED.
>'\u000B', U+000B VERTICAL TABULATION.
>'\f', U+000C FORM FEED.
>'\r', U+000D CARRIAGE RETURN.
>'\u001C', U+001C FILE SEPARATOR.
>'\u001D', U+001D GROUP SEPARATOR.
>'\u001E', U+001E RECORD SEPARATOR.
>'\u001F', U+001F UNIT SEPARATOR.
>' ',  U+0020 SPACE.
> (Note: that non-breaking space (\u00A0) is excluded)
>
> Character.isSpaceChar(codepoint) returns true if codepoint one of the 
> 

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[David Lloyd]

Sorry all, it looks like GMail doesn't know how to keep replies with
the thread when you change the subject line.  The follow-up to this
thread is 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/051960.html
with only a few small changes as discussed above.

On Fri, Mar 2, 2018 at 2:36 PM, David Lloyd  wrote:
> On Fri, Mar 2, 2018 at 2:34 PM, David Lloyd  wrote:
>> On Fri, Mar 2, 2018 at 12:49 PM, Xueming Shen  
>> wrote:
>>> Hi David,
>>>
>>> (1) Deflater.deflate(Bytebuffer)
>>>  the api doc regarding "no_flush" appears to be the copy/paste of the
>>> byte[] version
>>>  without being updated to the corresponding ByteBuffer?
>>
>> You're right, I missed that one.  I've incorporated this fix locally:
>
> Oops, this should have been:
>
> --- 8< --- cut here --- 8< ---
>
> diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java
> b/src/java.base/share/classes/java/util/zip/Deflater.java
> index 524125787a8..40f0d9736e2 100644
> --- a/src/java.base/share/classes/java/util/zip/Deflater.java
> +++ b/src/java.base/share/classes/java/util/zip/Deflater.java
> @@ -481,9 +481,9 @@ public class Deflater {
>   * in order to determine if more input data is required.
>   *
>   * This method uses {@link #NO_FLUSH} as its compression flush mode.
> - * An invocation of this method of the form {@code deflater.deflate(b)}
> + * An invocation of this method of the form {@code
> deflater.deflate(output)}
>   * yields the same result as the invocation of
> - * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
> + * {@code deflater.deflate(output, Deflater.NO_FLUSH)}.
>   *
>   * @param output the buffer for the compressed data
>   * @return the actual number of bytes of compressed data written to the
>
> --- 8< --- cut here --- 8< ---
>
> --
> - DML



-- 
- DML

Raw String Literal Library Support

[Jim Laskey]

With the announcement of JEP 326 Raw String Literals, we would like to open up 
a discussion with regards to RSL library support. Below are several implemented 
String methods that are believed to be appropriate. Please comment on those 
mentioned below including recommending alternate names or signatures. 
Additional methods can be considered if warranted, but as always, the bar for 
inclusion in String is high.

You should keep a couple things in mind when reviewing these methods.

Methods should be applicable to all strings, not just Raw String Literals.

The number of additional methods should be minimized, not adding every possible 
method.

Don't put any emphasis on performance. That is a separate discussion.

Cheers,

-- Jim

A. Line support.

public Stream lines()
Returns a stream of substrings extracted from this string partitioned by line 
terminators. Internally, the stream is implemented using a Spliteratorthat 
extracts one line at a time. The line terminators recognized are \n, \r\n and 
\r. This method provides versatility for the developer working with multi-line 
strings.
 Example:

String string = "abc\ndef\nghi";
Stream stream = string.lines();
List list = stream.collect(Collectors.toList());

 Result:

 [abc, def, ghi]


 Example:

String string = "abc\ndef\nghi";
String[] array = string.lines().toArray(String[]::new);

 Result:

 [Ljava.lang.String;@33e5ccce // [abc, def, ghi]


 Example:

String string = "abc\ndef\r\nghi\rjkl";
String platformString =
string.lines().collect(joining(System.lineSeparator()));

 Result:

 abc
 def
 ghi
 jkl


 Example:

String string = " abc  \n   def  \n ghi   ";
String trimmedString =
 string.lines().map(s -> s.trim()).collect(joining("\n"));

 Result:

 abc
 def
 ghi


 Example:

String table = `First Name  SurnamePhone
Al  Albert 555-
Bob Roberts555-
Cal Calvin 555-
   `;

// Extract headers
String firstLine = table.lines().findFirst​().orElse("");
List headings = List.of(firstLine.trim().split(`\s{2,}`));

// Build stream of maps
Stream> stream =
table.lines().skip(1)
 .map(line -> line.trim())
 .filter(line -> !line.isEmpty())
 .map(line -> line.split(`\s{2,}`))
 .map(columns -> {
 List values = List.of(columns);
 return IntStream.range(0, headings.size()).boxed()
 .collect(toMap(headings::get, 
values::get));
 });

// print all "First Name"
stream.map(row -> row.get("First Name"))
  .forEach(name -> System.out.println(name));

 Result:

 Al
 Bob
 Cal
B. Additions to basic trim methods. In addition to margin methods trimIndent 
and trimMarkers described below in Section C, it would be worth introducing 
trimLeft and trimRight to augment the longstanding trim method. A key question 
is how trimLeft and trimRight should detect whitespace, because different 
definitions of whitespace exist in the library. 

trim itself uses the simple test less than or equal to the space character, a 
fast test but not Unicode friendly. 

Character.isWhitespace(codepoint) returns true if codepoint one of the 
following;

   SPACE_SEPARATOR.
   LINE_SEPARATOR.
   PARAGRAPH_SEPARATOR.
   '\t', U+0009 HORIZONTAL TABULATION.
   '\n', U+000A LINE FEED.
   '\u000B', U+000B VERTICAL TABULATION.
   '\f', U+000C FORM FEED.
   '\r', U+000D CARRIAGE RETURN.
   '\u001C', U+001C FILE SEPARATOR.
   '\u001D', U+001D GROUP SEPARATOR.
   '\u001E', U+001E RECORD SEPARATOR.
   '\u001F', U+001F UNIT SEPARATOR.
   ' ',  U+0020 SPACE.
(Note: that non-breaking space (\u00A0) is excluded) 

Character.isSpaceChar(codepoint) returns true if codepoint one of the following;

   SPACE_SEPARATOR.
   LINE_SEPARATOR.
   PARAGRAPH_SEPARATOR.
   ' ',  U+0020 SPACE.
   '\u00A0', U+00A0 NON-BREAKING SPACE.
That sets up several kinds of whitespace; trim's whitespace (TWS), Character 
whitespace (CWS) and the union of the two (UWS). TWS is a fast test. CWS is a 
slow test. UWS is fast for Latin1 and slow-ish for UTF-16. 

We are recommending that trimLeft and trimRight use UWS, leave trim alone to 
avoid breaking the world and then possibly introduce trimWhitespace that uses 
UWS.

public String trim() 
Removes characters less than equal to space from the beginning and end of the 
string. No, change except spec clarification and links to the new trim methods.
Examples:
"".trim();  // ""
"   ".trim();   // ""
"  abc  

Re: [11] RFR: 8191410 : Unicode 10.0.0

[Rachna Goel]

Hi Roger, Ivan,

There is no change in algorithm as such but there has been mainly 
stability improvements, defects fixed, performance enhancements. e.g 
Updated header check, version and loading for latest binary files, 
Simple case mappings will be more efficient as unnecessary checking has 
been removed.

complete list of changes can be found at:

http://bugs.icu-project.org/trac/query?status=closed=fixed=fixedbyotherticket=60.1=60.2=project=999=id=summary=resolution=milestone=status=owner=type=priority=project=weeks=priority

Also, I have incorporated  suggestions given from Ivan.
Kindly have a look at updated webrev:

http://cr.openjdk.java.net/~rgoel/JDK-8191410/webrev.01/

Thanks,
Rachna


On 3/8/18 9:47 PM, Roger Riggs wrote:

Hi Rachna,

sun/text/normalizer/NormalizerImpl.java:

Is there a higher level description of the algorithmic changes?

102-105:   These look like accidental changes: the space should not be 
removed and the trailing "{" doesn't make sense in a comment.


Regards, Roger


On 3/8/2018 6:56 AM, Rachna Goel wrote:

Hi,

Please review the proposed changes for JDK-819410.

Bug : https://bugs.openjdk.java.net/browse/JDK-8191410

proposed changeset is located at :

http://cr.openjdk.java.net/~rgoel/JDK-8191410/webrev/

This serves as the implementation for JEP 327.





--
Thanks,
Rachna

Re: RFR JDK-8196748, tools/jar tests need to tolerate unrelated warnings

[Xueming Shen]

On 3/12/18, 11:57 PM, David Holmes wrote:


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

+ } else if (line.contains("Java HotSpot(TM) 
64-Bit Server VM warning:")) {
+ continue;  // ignore server vm warning 
see#8196748


That needs to be any VM warning as discussed.


Thanks David!  Will push with update for above comment.

http://cr.openjdk.java.net/~sherman/8196748/webrev/

Re: RFR JDK-8196748,tools/jar tests need to tolerate unrelated warnings

[David Holmes]

Hi Sherman,

On 13/03/2018 5:13 AM, Xueming Shen wrote:

On 3/11/18, 7:48 PM, David Holmes wrote:

Hi Sherman,


Thanks for working on this and adding the new functionality to 
OutputAnalyzer, but note that:


+   private static final String jvmwarningmsg =
+   "Java HotSpot\\(TM\\) 64-Bit Server VM warning:.*";

is Oracle JDK specific and 64-bit and server VM specific! Other tests 
use:


Pattern.compile(".*VM warning.*")

to exclude all VM warnings. Sorry that wasn't clear from previous 
discussions.


Hi David,

I was trying to be conservative to only filter out the "vm warnings" 
particularly for this
test failure. The webrev has been updated as suggested to filter out any 
line with

"VM warning".


The issue isn't about being conservative. You need to exclude the 
warnings no matter what VM the test is executed on. The original code 
would only pass on the Oracle JDK, and 64-bit Server VM. If run on 
OpenJDK the warning has the form:


OpenJDK 64-Bit Server VM warning: ...

If run on 32-bit there's no "64-bit". If run on Client VM there's no 
"Server".





Verified with the warning msg turned on as
http://cr.openjdk.java.net/~sherman/8196748/webrev.hs/src/hotspot/share/runtime/arguments.cpp.sdiff.html 


and ran the jtreg with -XX:+FastTLABRefill.


That will not hit all cases as it requires the -XX:+FastTLABRefill to 
be passed through to all JVMs launched by tests. The original problem 
occurred with unconditional warnings that did not depend on a 
particular flag being specified on the command-line. All of those 
cases have now been fixed however so just reenabling the message 
doesn't achieve anything.


The reason I did not try to turn "PrintSafepointStatisticsCount" back on 
(to reproduce) is that it was

not included in the changeset published in JDK-8196739 as
http://hg.openjdk.java.net/jdk/jdk/rev/74be5b4ed152

It appears the reason I failed to reproduce LeadingGarbage.java is the 
test in my repo does not forward
the vm options (I'm not sure why the test forwards those vm options in 
your repo). I have updated the test


I think there is a misunderstanding here. It doesn't matter what flag 
generates the warning - I modified PrintSafepointStatisticsCount as a 
convenience because the flags that originally triggered this problem no 
longer exist as flags (and so can't trigger the problem). The flag that 
generates the warning does not get specified as a VM argument to jtreg 
and does not get passed through to any other tools or VMs launched 
(there's no difference between our repos). That is the key point. In its 
original form (before the relevant flags were actually removed) we 
experience unconditional warnings from the VM and every execution of the 
VM (whether java, javac, jar, javap etc) is affected. Your test only 
triggered a warning due to the use of the FastTLABRefill flag and only 
the VM's that are explicitly passed that flag will encounter the 
warning. That excluded the VM (jar?) that actually triggers the test 
failure.


case to use the new test.lib (it was using the "old" jdk.testlibrary), 
and forward the vmoptions (this one

should have nothing to do with old libs or new libs though)


Right. Good to modernize the test, but not directly related to current 
issue.


Now the failure is reproducible and has been fixed with new method in 
OutputAnalyzer.java.


webrev: http://cr.openjdk.java.net/~sherman/8196748/webrev


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

+ } else if (line.contains("Java HotSpot(TM) 64-Bit 
Server VM warning:")) {

+ continue;  // ignore server vm warning see#8196748

That needs to be any VM warning as discussed.

---

test/lib/process/OutputAnalyzer.java

I'm not sure if "replace" is the right strategy here, if stderr may be 
very large. ?? The approach in JDK-8194067 was to see if the output 
lines matched the warning (though there are a couple of deficiencies in 
the way it did it).


Otherwise, my only minor concern is the assumption that the VM warnings 
will always be on stderr. That is the default, but it can be changed.


Thanks,
David


(arguments.cpp will not be put back, just to help reproduce)

Thanks,
Sherman