[9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057654
The idea is to separate construction logic and different checks
performed before/during method handle construction.
For example: move checks from MHs.foldArguments into MHs.foldArgumentChecks.
Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/
-ea -esa and COMPILE_THRESHOLD={0,30}.
Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com
Thanks!
Best regards,
Vladimir Ivanov
Re: test paths in repo
On Sep 4, 2014, at 9:55 PM, John Rose john.r.r...@oracle.com wrote: David Chase and I just noticed files like this in the JDK: http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/tip/test/java/util/stream/test/org/openjdk/tests/java/util/stream/ The package for the test code is org.openjdk.tests.java.util.stream. Although that's long, it is completely unambiguous. Generally, grammar of such paths appears to be: $REPO / test / $TEST_PACKAGE_PREFIX / $API_PACKAGE where TEST_PACKAGE_PREFIX is fixed as org/openjdk/tests, and API_PACKAGE is java/util/stream or some such. Further more, two other sets of classes are required to be on the bootclasspath and in the j.u.stream package: test framework classes; and white box tests. For the tests you mention, that don't need to be on the bootclass path we chose something different. It's named as if, perhaps in a better world?, most pure JDK java-based tests could be compiled and packaged into one jar file. FWIW there also happen to be a few other tests in other packages (e.g. SplittableRandomTest, that tests streams leveraging the test framework). These tests started out as pure testng tests and we used to run 'em via ant in the lambda repo, so i think that has influenced it's structure too (since IIRC it may have also contained more tests corresponding to other packages). FWIW a similar type of naming structure is used for JCK tests e.g.: package javasoft.sqe.tests.api.java.util.stream.IntStream; Has this organization for tests worn well? Would you do it again that way? I think it has worked out well so far, even if the naming is a little verbose (which is fine when using a compact package view in an IDE). I would do it again :-) Also, What is the reason for the close correspondence between the file system pathname and the Java package declaration? Is it so that IDEs can easily find the test files? (If so, which IDEs?) It definitely makes it easy for to tell an IDE, such as IntelliJ, these are my test source roots. We refactored the tests as least as much as we refactored the API/impl. When i refactor the API by making a name change i want my tests (+ supporting framework) to be automatically updated too, when i refactor the test framework i want my tests to automatically update. IMHO it is tricky to develop and maintain the JDK tests since it is hard to get a holistic view of all tests so one can crunch on 'em as a whole and doing useful analysis via IDEs that is possible on the JDK code itself (e.g. see recent cleanups for StringBuilder/Buffer). In this respect i would argue the current structure is actively hostile for maintaining tests, which i think should be given the same care and attention as the code they are testing. This is perhaps one reason why many tests duplicate assertion logic, there is some supporting help, but it's kind of a minor pain to use it and find it (perhaps that is just me being lazy?). Hth, Paul. For David's Arrays 2.0 work, which in some ways resembles the streams work, we want a good organization for tests, so we are looking at the streams tests as a template. Thanks for any insights... — John
[9] RFR (S): 8057657: Annotate LambdaForm parameters with types
http://cr.openjdk.java.net/~vlivanov/8057657/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057657
Add ability to annotate LambdaForm parameters with their types.
Type info could be useful during LambdaForm compilation to produce
better bytecode.
Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/
-ea -esa and COMPILE_THRESHOLD={0,30}.
Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com
Thanks!
Best regards,
Vladimir Ivanov
Re: RFR: JDK-8057556: JDP should better handle non-active interfaces
Hi Peter,
I fixed it and created new webrev.
http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.1/
Could you review it again?
Thanks,
Yasumasa
(2014/09/05 17:20), Peter Allwin wrote:
Looks like only the first Interface will be considered if no srcAddress is
provided (succeeded will be false and we will throw to exit the while loop). Is
this intended?
Thanks!
/peter
On 4 sep 2014, at 17:59, Yasumasa Suenaga yasue...@gmail.com wrote:
Hi all,
Thank you so much, Dmitry!
I've created webrev for it.
http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.0/
Please review.
Thanks,
Yasumasa
(2014/09/04 21:26), Dmitry Samersoff wrote:
Yasumasa,
The CR number is JDK-8057556
I'll care about it's integration.
-Dmitry
On 2014-09-02 18:52, Yasumasa Suenaga wrote:
Hi all,
I'm trying to use JDP on my Fedora20 machine.
My machine has two NICs and only one NIC is up.
I passed system properties as below, however JDP broadcaster
thread was not started:
-Dcom.sun.management.jmxremote.port=7091
-Dcom.sun.management.jmxremote.authenticate=false
-Dcom.sun.management.jmxremote.ssl=false
-Dcom.sun.management.jmxremote.autodiscovery=true
-Dcom.sun.management.jdp.name=TEST
I checked exceptions with jdb, SocketException was occurred in
JDPControllerRunner#run(), and it was caused by another NIC
is down.
Currently, DiagramChannel which is used in JDPBroadcaster
tries to send JDP packet through all UP NICs.
However, NIC which is controlled by NetworkManager seems to
be still UP when ifdown command is executed.
(It seems to be removed IP address from NIC only.)
This problem may be Fedora, however I think it should be
improved in JDK.
I've created a patch as below, and it works fine in my environment.
(jdk9/dev/jdk)
If this patch may be accepted, I will file this to JBS.
diff -r 68a6bb51cb26
src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
--- a/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
Mon Sep 01 13:33:28 2014 +0200
+++ b/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
Tue Sep 02 23:25:50 2014 +0900
@@ -35,6 +35,7 @@
import java.nio.ByteBuffer;
import java.nio.channels.DatagramChannel;
import java.nio.channels.UnsupportedAddressTypeException;
+import java.util.Enumeration;
/**
* JdpBroadcaster is responsible for sending pre-built JDP packet across a Net
@@ -79,6 +80,15 @@
if (srcAddress != null) {
// User requests particular interface to bind to
NetworkInterface interf =
NetworkInterface.getByInetAddress(srcAddress);
+
+if (interf == null) {
+throw new JdpException(Unable to get network interface for
+ srcAddress.toString());
+}
+
+if (!interf.isUp() || !interf.supportsMulticast()) {
+throw new JdpException(interf.getName() + does not support
multicast.);
+}
+
try {
channel.bind(new InetSocketAddress(srcAddress, 0));
} catch (UnsupportedAddressTypeException ex) {
@@ -86,6 +96,23 @@
}
channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, interf);
}
+else {
+EnumerationNetworkInterface nics =
NetworkInterface.getNetworkInterfaces();
+while (nics.hasMoreElements()) {
+NetworkInterface nic = nics.nextElement();
+
+if (nic.isUp() nic.supportsMulticast()) {
+try {
+
channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, nic);
+} catch (IOException ex) {
+System.err.println(WARNING: JDP broadcaster cannot use +
nic.getName() + : + ex.getMessage());
+}
+}
+
+}
+
+}
+
}
/**
Thanks,
Yasumasa
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654 +1 Paul.
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete
patch. Can we please try to systematically use more
readable/robust/secure idioms? E.g.:
a) Always have curly braces around the blocks?
if (ok ...) {
ok = false;
}
if (!ok) {
throw misMatchedTypes(...);
}
return rtype;
vs.
if (ok ...)
ok = false;
if (!ok)
throw misMatchedTypes(...);
return rtype;
Apple's goto fail; bug, anyone?
b) Have only a single initialization per line?
boolean match = true;
boolean fail = false;
vs.
boolean match = true, fail = false;
c) Always have parentheses in ternary operators predicates?
int foldVals = (rtype == void.class) ? 0 : 1;
vs.
int foldVals = rtype == void.class ? 0 : 1;
Thanks,
-Aleksey.
Re: [9] RFR (S) 8057656: Improve MethodType.isCastableTo() MethodType.isConvertibleTo() checks
On Sep 5, 2014, at 10:23 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
http://cr.openjdk.java.net/~vlivanov/8057656/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057656
854 if (!canConvert(returnType(), newType.returnType()))
855 return false;
856 Class?[] srcTypes = newType.ptypes;
857 Class?[] dstTypes = ptypes;
Are the src and dst the wrong way around?
srcTypes = ptypes
dstTypes = newType.ptypes
896 private static boolean canCast(Class? src, Class? dst) {
897 if (src.isPrimitive() !dst.isPrimitive()) {
898 if (dst == Object.class || dst.isInterface()) return true;
How come if the src is primitive and the dst is an interface it returns true
for any interface? I guess there are subtly different rules here for casting
and conversion.
Paul.
There are some corner cases which MT.isCastableTo() MT.isConvertibleTo()
don't treat right (e.g. int-String converstion of return type in
MT.isCastableTo()).
Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea
-esa and COMPILE_THRESHOLD={0,30}.
Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com
Thanks!
Best regards,
Vladimir Ivanov
Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF
Paul, thanks for review.
Generally looks good (re: Peter's observation of continue/break).
- LambdaFormEditor
61 private static final class Transform {
62 final long packedBytes;
63 final byte[] fullBytes;
64 final LambdaForm result; // result of transform, or null, if
there is none available
65
66 private enum Kind {
More an oddity really, Transform.Kind is private but exposed via package
private methods.
Good point. I'll fix that.
187 static Transform of(Kind k, int b1, byte[] b234) {
It might be worthwhile investing in a unit test for LambdaFormEditor to
exercise the transform types and transitions, otherwise edge cases might bite
us later.
Tests for JEP 210 (SQE is working on them) should cover that.
I was thinking Transform could be greatly simplified if one just uses byte[],
but IIUC (via JOL) that increases the instance size by 16 bytes, although that
may be small compared to the LambdaForm result and if say a WekRef is also used
to hold that result. Plus if that is the case the transformCache could be
simplified by just supporting Transform[] and ConcurrentHashMap. I guess the
underlying question i am asking here is do these space savings really give us
much over some less complicated code?
All these specializations can be considered overoptimizations, but I'd
prefer to leave them. Complexity is manageable, encapsulated, and
localized ([1],[2]).
And these specializations are for the common case. The numbers I got for
Octane shows that:
(1) large transforms are very rare:
98-99% of Transforms fit into packed representation;
(2) for LF caches
(a) 70-80% are single-element;
(b) 20-30% fit into array (16 elements)
(c) CHM is needed very rarely (1%)
Best regards,
Vladimir Ivanov
[1] http://cr.openjdk.java.net/~vlivanov/lfc/editor.no_packed
[2] http://cr.openjdk.java.net/~vlivanov/lfc/editor.no_single_cache
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
+1
On 05 Sep 2014, at 12:46, Aleksey Shipilev aleksey.shipi...@oracle.com wrote:
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete
patch. Can we please try to systematically use more
readable/robust/secure idioms? E.g.:
a) Always have curly braces around the blocks?
if (ok ...) {
ok = false;
}
if (!ok) {
throw misMatchedTypes(...);
}
return rtype;
vs.
if (ok ...)
ok = false;
if (!ok)
throw misMatchedTypes(...);
return rtype;
Apple's goto fail; bug, anyone?
b) Have only a single initialization per line?
boolean match = true;
boolean fail = false;
vs.
boolean match = true, fail = false;
c) Always have parentheses in ternary operators predicates?
int foldVals = (rtype == void.class) ? 0 : 1;
vs.
int foldVals = rtype == void.class ? 0 : 1;
Thanks,
-Aleksey.
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
To the style rant, I mean.
On 05 Sep 2014, at 13:40, Marcus Lagergren marcus.lagerg...@oracle.com wrote:
+1
On 05 Sep 2014, at 12:46, Aleksey Shipilev aleksey.shipi...@oracle.com
wrote:
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete
patch. Can we please try to systematically use more
readable/robust/secure idioms? E.g.:
a) Always have curly braces around the blocks?
if (ok ...) {
ok = false;
}
if (!ok) {
throw misMatchedTypes(...);
}
return rtype;
vs.
if (ok ...)
ok = false;
if (!ok)
throw misMatchedTypes(...);
return rtype;
Apple's goto fail; bug, anyone?
b) Have only a single initialization per line?
boolean match = true;
boolean fail = false;
vs.
boolean match = true, fail = false;
c) Always have parentheses in ternary operators predicates?
int foldVals = (rtype == void.class) ? 0 : 1;
vs.
int foldVals = rtype == void.class ? 0 : 1;
Thanks,
-Aleksey.
Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF
On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
Paul, thanks for review.
Generally looks good (re: Peter's observation of continue/break).
- LambdaFormEditor
61 private static final class Transform {
62 final long packedBytes;
63 final byte[] fullBytes;
64 final LambdaForm result; // result of transform, or null, if
there is none available
65
66 private enum Kind {
More an oddity really, Transform.Kind is private but exposed via package
private methods.
Good point. I'll fix that.
+1 on review.
187 static Transform of(Kind k, int b1, byte[] b234) {
It might be worthwhile investing in a unit test for LambdaFormEditor to
exercise the transform types and transitions, otherwise edge cases might
bite us later.
Tests for JEP 210 (SQE is working on them) should cover that.
Ok.
I was thinking Transform could be greatly simplified if one just uses
byte[], but IIUC (via JOL) that increases the instance size by 16 bytes,
although that may be small compared to the LambdaForm result and if say a
WekRef is also used to hold that result. Plus if that is the case the
transformCache could be simplified by just supporting Transform[] and
ConcurrentHashMap. I guess the underlying question i am asking here is do
these space savings really give us much over some less complicated code?
All these specializations can be considered overoptimizations, but I'd prefer
to leave them. Complexity is manageable, encapsulated, and localized
([1],[2]).
And these specializations are for the common case. The numbers I got for
Octane shows that:
(1) large transforms are very rare:
98-99% of Transforms fit into packed representation;
(2) for LF caches
(a) 70-80% are single-element;
(b) 20-30% fit into array (16 elements)
(c) CHM is needed very rarely (1%)
OK, good to see some numbers on this, quite reassuring.
Paul.
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
On 09/05/2014 12:31 PM, Paul Sandoz wrote: On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654 +1 Paul. *@SuppressWarnings(LocalVariableHidesMemberVariable)* AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight. cheers; Rémi
Re: [9] RFR (L): 8057042: LambdaFormEditor: derive new LFs from a base LF
Paul, Peter, Morris, thanks for review.
Best regards,
Vladimir Ivanov
On 9/5/14, 3:51 PM, Paul Sandoz wrote:
On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
Paul, thanks for review.
Generally looks good (re: Peter's observation of continue/break).
- LambdaFormEditor
61 private static final class Transform {
62 final long packedBytes;
63 final byte[] fullBytes;
64 final LambdaForm result; // result of transform, or null, if
there is none available
65
66 private enum Kind {
More an oddity really, Transform.Kind is private but exposed via package
private methods.
Good point. I'll fix that.
+1 on review.
187 static Transform of(Kind k, int b1, byte[] b234) {
It might be worthwhile investing in a unit test for LambdaFormEditor to
exercise the transform types and transitions, otherwise edge cases might bite
us later.
Tests for JEP 210 (SQE is working on them) should cover that.
Ok.
I was thinking Transform could be greatly simplified if one just uses byte[],
but IIUC (via JOL) that increases the instance size by 16 bytes, although that
may be small compared to the LambdaForm result and if say a WekRef is also used
to hold that result. Plus if that is the case the transformCache could be
simplified by just supporting Transform[] and ConcurrentHashMap. I guess the
underlying question i am asking here is do these space savings really give us
much over some less complicated code?
All these specializations can be considered overoptimizations, but I'd prefer
to leave them. Complexity is manageable, encapsulated, and localized ([1],[2]).
And these specializations are for the common case. The numbers I got for Octane
shows that:
(1) large transforms are very rare:
98-99% of Transforms fit into packed representation;
(2) for LF caches
(a) 70-80% are single-element;
(b) 20-30% fit into array (16 elements)
(c) CHM is needed very rarely (1%)
OK, good to see some numbers on this, quite reassuring.
Paul.
[9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ discussion:http://mail.openjdk.java.net/pipermail/net-dev/2014-August/008625.html Changes: - Removethe following methods: Integer::parseInt(CharSequence s, int radix, int beginIndex) Integer::parseUnsignedInt(CharSequence s, int radix, int beginIndex) Long::parseLong(CharSequence s, int radix, int beginIndex) Long::parseUnsignedLong(CharSequence s, int radix, int beginIndex) - Move radix parameter to the end in the following methods: Integer::parseInt(CharSequence s, int radix, int beginIndex, int endIndex) Integer::parseUnsignedInt(CharSequence s, int radix, int beginIndex, int endIndex) Long::parseLong(CharSequence s, int radix, int beginIndex, int endIndex) Long::parseUnsignedLong(CharSequence s, int radix, int beginIndex, int endIndex) - Change all places in the code where the methods already have been adopted - Add some tests for parseUnsignedInt/-Long - Ensure that when parsing fails the correct index is reported in the exception /Claes
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
On Sep 5, 2014, at 2:30 PM, Remi Forax fo...@univ-mlv.fr wrote: On 09/05/2014 12:31 PM, Paul Sandoz wrote: On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654 +1 Paul. *@SuppressWarnings(LocalVariableHidesMemberVariable)* AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight. I nearly called that out too (sitting on the fence a little in this respect). Might be worth doing as a separate pass. Paul.
Re: [9] RFR (S) 8050173: Generalize BMH.copyWith API to all method handles
http://cr.openjdk.java.net/~vlivanov/8050173/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8050173
Added j.l.i.MethodHandle.copyWith(MethodType, LambdaForm) and provided
implementation for all subclasses.
Also, some cleanups:
* rewrote MH.viewAsType on top of MH.copyWith;
* extended MH.viewAsType to do strict checks w/ assertions turned on (new
parameter: boolean strict);
* extended MT.isViewableAs to accept both interface preserving and interface
erasing conversions (new parameter: boolean keepInterfaces).
Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea
-esa and COMPILE_THRESHOLD={0,30}.
Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com
Looks good, just one comment.
MethodHandles.restrictReceiver
This method has:
1578 private MethodHandle restrictReceiver(MemberName method, MethodHandle
mh, Class? caller) throws IllegalAccessException {
...
1589 assert(mh instanceof DirectMethodHandle); //
DirectMethodHandle.copyWith
Why not make the second parameter be DirectMethodHandle mh ?
Good point! While prototyping this I spotted uncovered corner case
(restrict a receiver on a MH with bound caller).
Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8050173/webrev.01/
Diff:
http://cr.openjdk.java.net/~vlivanov/8050173/webrev.00.01/
Reordered restrictReceiver and maybeBindCaller operations.
Best regards,
Vladimir Ivanov
Re: [9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
Paul, Remi, thanks for review. Renamed type - mtype removed @SuppressWarnings. Updated webrev in place. Best regards, Vladimir Ivanov On 9/5/14, 5:13 PM, Paul Sandoz wrote: On Sep 5, 2014, at 2:30 PM, Remi Forax fo...@univ-mlv.fr wrote: On 09/05/2014 12:31 PM, Paul Sandoz wrote: On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com wrote: http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654 +1 Paul. *@SuppressWarnings(LocalVariableHidesMemberVariable)* AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight. I nearly called that out too (sitting on the fence a little in this respect). Might be worth doing as a separate pass. Paul.
Re: [9] RFR (S) 8050173: Generalize BMH.copyWith API to all method handles
On Sep 5, 2014, at 3:15 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
Looks good, just one comment.
MethodHandles.restrictReceiver
This method has:
1578 private MethodHandle restrictReceiver(MemberName method,
MethodHandle mh, Class? caller) throws IllegalAccessException {
...
1589 assert(mh instanceof DirectMethodHandle); //
DirectMethodHandle.copyWith
Why not make the second parameter be DirectMethodHandle mh ?
Good point! While prototyping this I spotted uncovered corner case (restrict
a receiver on a MH with bound caller).
Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8050173/webrev.01/
Diff:
http://cr.openjdk.java.net/~vlivanov/8050173/webrev.00.01/
Reordered restrictReceiver and maybeBindCaller operations.
Looks good,
Paul.
Re: [9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
On 05/09/2014 14:03, Claes Redestad wrote: Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ Thanks for doing this, I think the API is much better now and much less error prone. The drive-by fix to the index in the NumberFormatException also looks good. -Alan.
Re: [9] RFR (S) 8057656: Improve MethodType.isCastableTo() MethodType.isConvertibleTo() checks
http://cr.openjdk.java.net/~vlivanov/8057656/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8057656
854 if (!canConvert(returnType(), newType.returnType()))
855 return false;
856 Class?[] srcTypes = newType.ptypes;
857 Class?[] dstTypes = ptypes;
Are the src and dst the wrong way around?
srcTypes = ptypes
dstTypes = newType.ptypes
No, they are right. Parameters and return type conversions have opposite
directions.
896 private static boolean canCast(Class? src, Class? dst) {
897 if (src.isPrimitive() !dst.isPrimitive()) {
898 if (dst == Object.class || dst.isInterface()) return true;
How come if the src is primitive and the dst is an interface it returns true
for any interface? I guess there are subtly different rules here for casting
and conversion.
There are 2 types of converstions: MH.asType() and
MHs.explicitCastArguemnts() with more relaxed semantics.
One of the differences is that interfaces are coerced to Object, since
verifier allows any interface to be treated as Object.
Your questions reminded me about related changes waiting in the queue
and I decided to include then here.
Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8057656/webrev.01/
Got rid of MT.isCastableTo(). MHs.explicitCastEquivalentToAsType() is
used instead. Also, it has detailed overview of differences between
MT.asType() and MHs.explicitCastArguments().
Best regards,
Vladimir Ivanov
Paul.
There are some corner cases which MT.isCastableTo() MT.isConvertibleTo() don't
treat right (e.g. int-String converstion of return type in MT.isCastableTo()).
Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea
-esa and COMPILE_THRESHOLD={0,30}.
Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com
Thanks!
Best regards,
Vladimir Ivanov
Re: [9] RFR (S) 8050173: Generalize BMH.copyWith API to all method handles
On Jul 16, 2014, at 1:50 AM, Paul Sandoz paul.san...@oracle.com wrote: Why not make the second parameter be DirectMethodHandle mh ? Good suggestion; thanks. Makes the restrictReceiver logic less magic. — John
[9] Review request : JDK-8057719: Develop new tests for LambdaForm Reduction and Caching feature
Hello, Please review the new tests for the feature Lambda Form Reduction and Caching https://bugs.openjdk.java.net/browse/JDK-8046703 JBS task: https://bugs.openjdk.java.net/browse/JDK-8057719 Webrev: http://cr.openjdk.java.net/~kshefov/8057719/webrev.00/ These tests also depend on testlibrary change: https://bugs.openjdk.java.net/browse/JDK-8057707 Webrev of the testlib change: http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/ Thanks -Konstantin
[9] Review request : JDK-8057707: TEST library enhancement: copy sun.hotspot.whitebox classes from hotspot repo and enhance lib/testlibrary/jsr292/com/oracle/testlibrary/jsr292/Helper.java
Hello, Please review the change in testlibrary https://bugs.openjdk.java.net/browse/JDK-8057707 This change is needed for new tests for the feature Lambda Form Reduction and Caching https://bugs.openjdk.java.net/browse/JDK-8046703 Webrev of the testlibrary change: http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/ JBS entry for new test dev is https://bugs.openjdk.java.net/browse/JDK-8057719 Webrev for new tests is http://cr.openjdk.java.net/~kshefov/8057719/webrev.00/ Thanks -Konstantin
Re: Impact of code difference in Collection#contains() worth improving?
On Fri, Aug 29, 2014 at 7:53 PM, Guy Steele guy.ste...@oracle.com wrote: But I cannot resist recalling that one of the earliest pieces of software in the implementation of EMACS (back when the implementation language was TECO, a text-editing language) was a routine that, when it loaded TECO macros from a file, would carefully remove comments and excess whitespace in order to improve the execution speed of the macros (and therefore the response time of the EMACS keystrokes)! We have come a long, long way in 38 years. I often think we haven't made enough progress on the basic things. Worrying about comments in our TECO macros is a lot like worrying about dead assertion code in our compiled bytecode. And we are still plagued by fixed size native stacks, the 64k bytecode size limit, unreliable Unix signals, and of course, unavailability of tail recursion.
[8u40] RFR 6642881: Improve performance of Class.getClassLoader()
Summary: Add classLoader to java/lang/Class instance for fast access This is a backport request for 8u40. This change has been in the jdk9 code for 3 months without any problems. The JDK changes hg imported cleanly. The Hotspot change needed a hand merge for create_mirror call in klass.cpp. http://cr.openjdk.java.net/~coleenp/6642881_8u40_jdk/ http://cr.openjdk.java.net/~coleenp/6642881_8u40_hotspot/ bug link https://bugs.openjdk.java.net/browse/JDK-6642881 Ran jdk_core jtreg tests in jdk with both jdk/hotspot changes. Also ran jck java_lang tests with only the hotspot change. The hotspot change can be tested separately from the jdk change (but not the other way around). Thanks, Coleen
Re: [9] Review request : JDK-8057707: TEST library enhancement: copy sun.hotspot.whitebox classes from hotspot repo and enhance lib/testlibrary/jsr292/com/oracle/testlibrary/jsr292/Helper.java
On 05/09/2014 18:57, Konstantin Shefov wrote: Hello, Please review the change in testlibrary https://bugs.openjdk.java.net/browse/JDK-8057707 This change is needed for new tests for the feature Lambda Form Reduction and Caching https://bugs.openjdk.java.net/browse/JDK-8046703 Webrev of the testlibrary change: http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/ It seems a bit strange, are you sure this is right thing to do? I would have thought that tests using the hotspot whitebox library would be pushed to the hotspot/test tree. -Alan
Re: RFR (JAXP) 8056202: Xerces Update: Catalog Resolver
Hi Joe, This seems OK Best, Lance On Aug 28, 2014, at 10:46 PM, huizhe wang huizhe.w...@oracle.com wrote: Hi, This is an update to Xerces' Catalog Resolver implementation. The changes were mostly performance related, for example the changes to the normalizeURI method in Catalog.java to avoid creating new Strings if the input is already in normalized form. Bug: https://bugs.openjdk.java.net/browse/JDK-8056202 Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8056202/webrev/ Thanks, Joe 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: RFR (JAXP) 8056202: Xerces Update: Catalog Resolver
Thanks Lance! Joe On 9/5/2014 1:44 PM, Lance Andersen wrote: Hi Joe, This seems OK Best, Lance On Aug 28, 2014, at 10:46 PM, huizhe wang huizhe.w...@oracle.com mailto:huizhe.w...@oracle.com wrote: Hi, This is an update to Xerces' Catalog Resolver implementation. The changes were mostly performance related, for example the changes to the normalizeURI method in Catalog.java to avoid creating new Strings if the input is already in normalized form. Bug: https://bugs.openjdk.java.net/browse/JDK-8056202 Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8056202/webrev/ http://cr.openjdk.java.net/%7Ejoehw/jdk9/8056202/webrev/ Thanks, Joe http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifhttp://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com
Re: [9] Review request : JDK-8057707: TEST library enhancement: copy sun.hotspot.whitebox classes from hotspot repo and enhance lib/testlibrary/jsr292/com/oracle/testlibrary/jsr292/Helper.java
These test are for core-libs feature, but we want to test that unused lambda forms are garbage collected using WhiteBox.fullGC() method. There are three tests that use one common class, only one of them uses whitebox api. So it is hard to move one of tests to hotspot repo. - Konstantin Исходное сообщение От: Alan Bateman alan.bate...@oracle.com Дата:06.09.2014 0:38 (GMT+04:00) Кому: Konstantin Shefov konstantin.she...@oracle.com,VLADIMIR.X.IVANOV vladimir.x.iva...@oracle.com,core-libs-dev@openjdk.java.net,mlvm-...@openjdk.java.net Тема: Re: [9] Review request : JDK-8057707: TEST library enhancement: copy sun.hotspot.whitebox classes from hotspot repo and enhance lib/testlibrary/jsr292/com/oracle/testlibrary/jsr292/Helper.java On 05/09/2014 18:57, Konstantin Shefov wrote: Hello, Please review the change in testlibrary https://bugs.openjdk.java.net/browse/JDK-8057707 This change is needed for new tests for the feature Lambda Form Reduction and Caching https://bugs.openjdk.java.net/browse/JDK-8046703 Webrev of the testlibrary change: http://cr.openjdk.java.net/~kshefov/8057707/webrev.00/ It seems a bit strange, are you sure this is right thing to do? I would have thought that tests using the hotspot whitebox library would be pushed to the hotspot/test tree. -Alan
Re: Impact of code difference in Collection#contains() worth improving?
On Aug 30, 2014, at 7:17 AM, Ulf Zibis ulf.zi...@cosoco.de wrote: Am 30.08.2014 um 01:33 schrieb John Rose: On Aug 29, 2014, at 1:05 PM, Ulf Zibis ulf.zi...@cosoco.de mailto:ulf.zi...@cosoco.de wrote: Thanks for explaining this, but a very little nit: the immediate (I.e. -1) uses additional 32/64 bits in code which must be loaded from memory and wastes space in CPU cache or am I wrong? This could be saved with = 0. I have to say you're more wrong than right about this. Optimizers routinely change the form of constants. For example, a constant 0 will often show up as something like xor eax,eax, not a 32-bit literal zero that loads from somewhere in memory. A comparison of the form x -1 will be freely changed to x = 0 and back again; the latter form may (or may not, depending on chip version) transform to test eax, with no -1 or 0 in sight. 1. Thanks for the hint about x -1 === x = 0. But I'm afraid this would apply on the x != -1 case we are discussing here. The equality comparison would be less likely to transform to a non-equality comparison. But it is still possible in principle, if the JIT could prove that values x -1 are impossible. 2. Are you really sure this optimization is always implemented, as following bug is still open: JDK-6984886 : Transform comparisons against odd border to even border http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6984886 Well, I'm pretty sure that no optimizations is always implemented; there are usually corner cases which prevent optimizations. There is always a possibility of deoptimization to the interpreter, for example. And there are lots of nice-to-have optimizations that don't make a difference to real applications. Much of compiler development is driven by observing actual speed bottlenecks and removing them. But the bug reference is useful; thanks! I added a comment to show where the optimization occurs now—disappointingly little I grant you—and where to start looking to improve it. The bug is rated P5 (lowest rating) probably because the reporter said it should be faster; wouldn't this be nice is a far weaker argument than I'm spending too much on hardware because this loop is slow. My comments about the unpredictability of optimizers still stand. The most robust way to manage such problems is, first confirm it is a real performance problem (not a micro-nano thing), and second get the optimizer to make all equivalent inputs produce good code. The tiny place where this optimization is done in HotSpot suggests that that was a place where the transform actually mattered the most. — John
Re: Impact of code difference in Collection#contains() worth improving?
On Aug 30, 2014, at 10:58 AM, Doug Lea d...@cs.oswego.edu wrote: In the present case, I'm with Martin about short-circuiting this with a simple approximate answer: Rather than flip a coin choosing between solutions A and B, pick the one with smaller bytecode. This has a decent enough correlation with actual performance factors to better than chance. And even if the effects are small, sometimes the only path to making things substantially faster is a few percent at a time. I grant you that's better than a coin flip. Contributions from the constant pool and various attributes are going to be pretty noisy and unrelated to instruction traces. You'll get somewhat less random information from measuring the affected method's bytecodes. (Which is hard to do without writing an ASM-based tool, and compared with wc -c Foo.class.) — John
