Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible
outside (MTF and MT.form() are package-private).
Best regards,
Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks,
Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal
read sees an outdated null value, a new LambdaForm is created and
setCachedLambdaForm(...) is executed. This will guarantee that the
non-null value is seen and used. The unnecessary creation of a new
LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find
the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks,
Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann
tobias.hartm...@oracle.com wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Yes, that is how I see it. The fast path is a racy non-volatile read
of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be
broken. But the JMM is carefully designed to support safe publication
of array elements, and through array elements.)
— John
Re: RFR [8037866] Replace the Fun class in tests with lambdas
Thank you Martin for review! On 02.06.2014 8:36, Martin Buchholz wrote: Thanks! Looks good. There might be more common infrastructure to take advantage of later. @FunctionalInterface is not really in the spirit of my super-compact stylistically odd test code, but OK. I'll remove the annotation to keep the spirit :) Sincerely yours, Ivan On Sun, Jun 1, 2014 at 5:57 PM, Ivan Gerasimov ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote: Hello! Would you please help review the fix, which replaces the auxiliary Fun class with the same named functional interface? The anonymous classes instantiations are replaced with lambdas. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8037866 WEBREV: http://cr.openjdk.java.net/~igerasim/8037866/0/webrev/ http://cr.openjdk.java.net/%7Eigerasim/8037866/0/webrev/ Sincerely yours, Ivan
Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache
Tobias, I'll take care of it.
Best regards,
Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible
outside (MTF and MT.form() are package-private).
Best regards,
Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks,
Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal
read sees an outdated null value, a new LambdaForm is created and
setCachedLambdaForm(...) is executed. This will guarantee that the
non-null value is seen and used. The unnecessary creation of a new
LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find
the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks,
Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann
tobias.hartm...@oracle.com wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Yes, that is how I see it. The fast path is a racy non-volatile read
of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be
broken. But the JMM is carefully designed to support safe publication
of array elements, and through array elements.)
— John
Re: RFR: JDK-8044343 - Check jdk/src/windows/native/java/lang/ProcessEnvironment_md.c for JNI pending exceptions
On 31/05/2014 17:33, Mark Sheppard wrote: Hi, please oblige and review the following change http://cr.openjdk.java.net/~msheppar/8044343/webrev/ which addresses issue https://bugs.openjdk.java.net/browse/JDK-8044343 which is a backport of https://bugs.openjdk.java.net/browse/JDK-8036603 the original changeset didn't apply cleanly and was applied manually http://hg.openjdk.java.net/jdk9/dev/jdk/rev/323b64a9dede This looks okay to me and should have the 8u version the same as 9 (except for the additional blank lines in the 9 version). -Alan
Re: RFR: 8041602 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
On 31/05/2014 23:02, Mark Sheppard wrote: Hi, please oblige and review the following change http://cr.openjdk.java.net/~msheppar/8041602/webrev/ which addresses the issue https://bugs.openjdk.java.net/browse/JDK-8041602 which is a backport of https://bugs.openjdk.java.net/browse/JDK-8035340 the original changeset didn't apply cleanly and was applied manually http://hg.openjdk.java.net/jdk9/dev/jdk/rev/740ffd98e35a This looks okay to me, I'm curious as whether it was just the new location in the jdk9/jdk tree that caused the issue or another change. A minor comment on L48 where it could be if (result .. to be consistent. -Alan
Re: RFR for 8043740 (Doubles with large exponents overflow to Infinity incorrectly)
Hi all -
I've made a quick revision to that last patch. Please find inline the
latest link + patch.
http://www.sandipan.net/public/webrevs/8043740/webrev.01/
Cheers,
SR
--- old/src/share/classes/sun/misc/FloatingDecimal.java 2014-06-02
09:32:20.0 -0400
+++ new/src/share/classes/sun/misc/FloatingDecimal.java 2014-06-02
09:32:19.0 -0400
@@ -1994,19 +1994,29 @@
}
int expLimit = BIG_DECIMAL_EXPONENT+nDigits+nTrailZero;
if ( expOverflow || ( expVal expLimit ) ){
-//
-// The intent here is to end up with
-// infinity or zero, as appropriate.
-// The reason for yielding such a small decExponent,
-// rather than something intuitive such as
-// expSign*Integer.MAX_VALUE, is that this value
-// is subject to further manipulation in
-// doubleValue() and floatValue(), and I don't want
-// it to be able to cause overflow there!
-// (The only way we can get into trouble here is for
-// really outrageous nDigits+nTrailZero, such as
2 billion. )
-//
-decExp = expSign*expLimit;
+// There is still a chance that the exponent will be
+// safe to use: if it would eventually decrease
+// due to a negative decExp, and that number is below the limit.
+// We check for that here.
+if ( ( expSign == 1 decExp 0 )
+ ( expVal + decExp ) expLimit ) {
+ // Cannot overflow - adding a positive and negative number.
+ decExp = expVal + decExp;
+} else {
+ //
+ // The intent here is to end up with
+ // infinity or zero, as appropriate.
+ // The reason for yielding such a small decExponent,
+ // rather than something intuitive such as
+ // expSign*Integer.MAX_VALUE, is that this value
+ // is subject to further manipulation in
+ // doubleValue() and floatValue(), and I don't want
+ // it to be able to cause overflow there!
+ // (The only way we can get into trouble here is for
+ // really outrageous nDigits+nTrailZero, such as 2 billion. )
+ //
+ decExp = expSign*expLimit;
+}
} else {
// this should not overflow, since we tested
// for expVal (MAX+N), where N = abs(decExp)
--- /dev/null 2014-06-02 09:32:21.0 -0400
+++ new/test/java/lang/Double/Bug8043740.java 2014-06-02
09:32:20.0 -0400
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2001, 2014, 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 8043740
+ * @summary Test for Double.parseDouble incorrect normalization of exponent.
+ */
+public class Bug8043740 {
+
+public static void main(String[] args) {
+String text = 00.01e326;
+ double expected = Double.parseDouble(1e308);
+double value = Double.parseDouble(text);
+if (Double.compare(expected, value) != 0) {
+throw new RuntimeException(String [ + text +
+] was incorrectly parsed to [ + value + ]);
+}
+}
+
+}
Sandipan Razzaque | www.sandipan.net
On Sun, Jun 1, 2014 at 10:55 PM, Sandipan Razzaque m...@sandipan.net wrote:
Hi Brian/all,
Fix for the above-mentioned bug is complete. If you (or anyone else as
appropriate) could please review the changes and let me know of your
comments that would be great!
Link for the webrev:
http://www.sandipan.net/public/webrevs/8043740/webrev.00/
The patch is inlined below my signature.
Cheers,
SR
--- old/src/share/classes/sun/misc/FloatingDecimal.java 2014-06-01
22:33:34.947651253 -0400
+++ new/src/share/classes/sun/misc/FloatingDecimal.java 2014-06-01
22:33:34.827651251 -0400
@@ -1992,7 +1992,10 @@
break expLoop; // stop parsing exponent.
}
}
-int expLimit =
Re: RFR: 5043030 (reflect) unnecessary object creation in reflection
Hi David, et.al. I would let the compiler do auto-boxing where necessary. (Assuming object identity is not necessary). If type disambiguation is necessary then use a cast to the target type and let the compiler do the rest. It keeps the source code simple and readable. But I don't think it is worth a proactive pervasive change. The gain is overshadowed by the overhead of the reviews. $.02, Roger On 6/1/2014 11:07 PM, David Holmes wrote: Hi Andrej, Sorry for the delay getting back to you. On 29/05/2014 10:24 PM, Andrej Golovnin wrote: Hi David, The valueOf calls may also allocate a new object so you can't just delete the JvmtiExport::post_vm_object_alloc call. Unfortunately you can't tell whether a new object was allocated or not. It is only for the smaller primitive types that any kind of Object caching is mandated. It is only for the smaller values (-128 to +127) of the integer primitives types (plus boolean) that caching is mandated. Float.valueOf and Double.valueOf always create objects. You are right, that #valueOf call may allocate an object. But as far as I understand currently the JvmtiExport::post_vm_object_alloc call is only needed, because today the native code itself allocates an object by calling java_lang_boxing_object::create(type, value, CHECK_NULL);. Right, sorry - I was misunderstanding the purpose of the post_vm_object_alloc call: http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#VMObjectAlloc So from the perspective that you are diverting this back to Java code the hotspot changes look okay to me. The more general question, for the core-libs folk, is whether changing everything to use valueOf is overkill (given the limits of the required caching mechanisms) or good to do from a consistency perspective. I'm slightly on the overkill side of things but not enough to reject things. On the performance/benefit side, if I read things correctly you only see the 9GB of Boolean objects because you disable reflection-inflation - is that right? In that case, as Joel states, the gains are not really general, but on the other hand I don't see anything wrong with trying to improve the general efficiency here even if the greatest benefit comes from a non-mainstream usecase. David - My code changes this behavior and delegates object allocation back to Java by calling JavaCalls::call_static(boxed_value, klass_handle, vmSymbols::valueOf_name(), valueOf_signature, args, THREAD); But maybe I misunderstood the implementation of JavaCalls. Best regards, Andrej Golovnin
Re: RFR: 8041602 - (prefs) Check jdk/src/windows/native/java/util/WindowsPreference.c for JNI pending exceptions
Hi Alan, yes the differences in location was an issue and the jdk8 CoreLibraries.gmk had a clash regards Mark On 02/06/2014 14:37, Alan Bateman wrote: On 31/05/2014 23:02, Mark Sheppard wrote: Hi, please oblige and review the following change http://cr.openjdk.java.net/~msheppar/8041602/webrev/ which addresses the issue https://bugs.openjdk.java.net/browse/JDK-8041602 which is a backport of https://bugs.openjdk.java.net/browse/JDK-8035340 the original changeset didn't apply cleanly and was applied manually http://hg.openjdk.java.net/jdk9/dev/jdk/rev/740ffd98e35a This looks okay to me, I'm curious as whether it was just the new location in the jdk9/jdk tree that caused the issue or another change. A minor comment on L48 where it could be if (result .. to be consistent. -Alan
Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache
On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
Tobias, I'll take care of it.
Are you also taking care of the backports?
Best regards,
Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible
outside (MTF and MT.form() are package-private).
Best regards,
Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks,
Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the normal
read sees an outdated null value, a new LambdaForm is created and
setCachedLambdaForm(...) is executed. This will guarantee that the
non-null value is seen and used. The unnecessary creation of a new
LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS. Please find
the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks,
Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann
tobias.hartm...@oracle.com wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Yes, that is how I see it. The fast path is a racy non-volatile read
of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String would be
broken. But the JMM is carefully designed to support safe publication
of array elements, and through array elements.)
— John
Re: RFR: JDK-8044206: LambdaMetafactory.altMetafactory javadoc refers to wrong method
+1 On Jun 2, 2014, at 1:24 PM, Alexander Zuev alexander.z...@oracle.com wrote: Hello, please, review my fix for the issue JDK-8044206: LambdaMetafactory.altMetafactory javadoc refers to wrong method Link to the issue: https://bugs.openjdk.java.net/browse/JDK-8044206 Webrev: http://cr.openjdk.java.net/~kizune/8044206/webrev.00 The fix is trivial, the JavaDoc for the LambdaMetafactory.altMetafactory() stated that it's more advanced version of the more streamlined method and linked itself instead of the method LambdaMetafactory.metafactory() /Alex
Re: [9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache
Hi Vladimir,
On 02.06.2014 19:38, Vladimir Ivanov wrote:
On 6/2/14 9:23 PM, Christian Thalinger wrote:
On Jun 2, 2014, at 6:29 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com wrote:
Tobias, I'll take care of it.
Are you also taking care of the backports?
Yes. I'll backport into 8u myself and for 7 I'll ask help from
Sustaining.
Thank you!
Best,
Tobias
Best regards,
Vladimir Ivanov
Best regards,
Vladimir Ivanov
On 6/2/14 10:04 AM, Tobias Hartmann wrote:
On 28.05.2014 12:48, Vladimir Ivanov wrote:
Looks good.
It should be safe to sync on MTF instance since it's not accessible
outside (MTF and MT.form() are package-private).
Best regards,
Vladimir Ivanov
Thank you, Vladimir.
Could someone please push the patch?
Thanks,
Tobias
On 5/28/14 1:49 PM, Tobias Hartmann wrote:
Hi,
thanks everyone for the feedback!
@Remi: I agree with Paul. This is not a problem because if the
normal
read sees an outdated null value, a new LambdaForm is created and
setCachedLambdaForm(...) is executed. This will guarantee that the
non-null value is seen and used. The unnecessary creation of a new
LamdaForm is not a problem either.
@John: I added the code that you suggested to simulate CAS.
Please find
the new webrev at:
http://cr.openjdk.java.net/~anoll/8005873/webrev.02/
Sorry for the delay, I was on vacation.
Thanks,
Tobias
On 19.05.2014 20:31, John Rose wrote:
On May 16, 2014, at 4:56 AM, Tobias Hartmann
tobias.hartm...@oracle.com wrote:
Is it sufficient then to use synchronized (lambdaForms) { ... } in
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?
Yes, that is how I see it. The fast path is a racy non-volatile
read
of a safely-published structure.
(If safe publication via arrays were broken, java.lang.String
would be
broken. But the JMM is carefully designed to support safe
publication
of array elements, and through array elements.)
— John
