Re: 8026049: (bf) Intrinsify ByteBuffer.put{Int, Double, Float, ...} methods

I'm glad to see this coming along… The code, javadoc, and spec all look good. On Mar 20, 2015, at 3:00 PM, Vladimir Kozlov wrote: > >> http://cr.openjdk.java.net/~aph/unaligned.jdk.7/ >> > > My only comment for jdk change is to use bigEndian

Re: [9] RFR of 8065556: (bf) Buffer.position and other methods should include detail in IAE

Yes, that's still a good idea, until we fix JDK-6316156 . Thanks, Martin. — John On Mar 20, 2015, at 6:36 PM, Martin Buchholz wrote: > > The message construction code takes many bytecodes, and best practice a few > years ago was to segregate c

Re: Fw: Re: RFR [9] 8071472: Add field access to support, setting final fields in readObject

The reason for this part of the spec, is backward compatible evolvability of serial form. It allows an earlier implementation of a class to deserialize serial form from a later implementation that includes additional fields or values written to the stream, and allow them to be discarded by Ob

Re: [9] RFR of 8065556: (bf) Buffer.position and other methods should include detail in IAE

The message construction code takes many bytecodes, and best practice a few years ago was to segregate cold code like this into a separate method, and that's probably still true. Follow the practice of other classes and define private String outOfBoundsMsg(int position) On Fri, Mar 20, 2015 at 3

[9] RFR of 8065556: (bf) Buffer.position and other methods should include detail in IAE

Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8065556 Patch: http://cr.openjdk.java.net/~bpb/8065556/webrev.00/ My doubts concern the changes to the test portion, viz.: 1) Is the style sufficiently consistent? 2) I have not added a “@bug” tag like we normal

Re: RFR 9: 8071480: ProcessBuilder.start spec linked to the wrong checkRead and checkWrite methods

Thanks! This one is a real improvement! On Fri, Mar 20, 2015 at 2:33 PM, Roger Riggs wrote: > Please review two javadoc updates to java.lang.ProcessBuilder. > > Webrev: > http://cr.openjdk.java.net/~rriggs/webrev-typo-8073220/ > > Issues: > 8071480: (process spec) ProcessBuilder.start spec l

Re: RFR 9: 8067796: (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null

I probably implemented it the way it is intentionally. I would leave it. Pedantic changes such as this one won't make any users happy, but we're doing this elsewhere like in java.util.concurrent so I guess it's OK. But there's a much simpler way to implement it. Just move long remainingNanos =

Re: 8026049: (bf) Intrinsify ByteBuffer.put{Int, Double, Float, ...} methods

On 3/20/15 9:08 AM, Andrew Haley wrote: Webrev at http://cr.openjdk.java.net/~aph/unaligned.hotspot.7/ I don't see changes to VM's makefiles to set VM_BIG_ENDIAN. Or consider it is set if VM_LITTLE_ENDIAN is not set in prims/unsafe.cpp code. In globals.hpp the empty line after flag descript

RFR 9: 8067796: (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null

Please review this change to Process.waitFor(timeout, TimeUnit) to check for TimeUnit == null before checking for exited = true and timeout <= 0. The current implementation does not throw NPE if the timeout is <= 0 or the Process has already exited. The check for the validity of the arguments s

Re: [9] RFR of 8075655: NIO test generation scripts have incorrect path to Spp.java

Hi Brian, Look good, Roger On 3/20/2015 5:32 PM, Brian Burkhalter wrote: Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8075655 Patch: http://cr.openjdk.java.net/~bpb/8075655/webrev.00/ Change is to correct the path. Thanks, Brian

RFR 9: 8071480: ProcessBuilder.start spec linked to the wrong checkRead and checkWrite methods

Please review two javadoc updates to java.lang.ProcessBuilder. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-typo-8073220/ Issues: 8071480: (process spec) ProcessBuilder.start spec linked to the wrong checkRead and checkWrite methods 8073220: A typo in the documentation for class Pro

[9] RFR of 8075655: NIO test generation scripts have incorrect path to Spp.java

Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8075655 Patch: http://cr.openjdk.java.net/~bpb/8075655/webrev.00/ Change is to correct the path. Thanks, Brian

Re: RFR 8071667 : HashMap.computeIfAbsent() adds entry that HashMap.get() does not find.

Here's the latest: http://cr.openjdk.java.net/~bchristi/8071667/webrev.4/ That should have everything. Thanks again, -Brent On 3/20/15 2:52 AM, Paul Sandoz wrote: On Mar 19, 2015, at 11:31 PM, Brent Christian wrote: Hi, Paul Thank you for the suggested doc adjustments. They're applied h

Re: RFR [9] : Add default[Read|Write]Object to java.util.Date

On 20/03/2015 18:55, Chris Hegarty wrote: Trivial change that came up a few months ago, and again recently, that java.util.Date should call default[Read|Write]Object to strictly conform to the Serialization spec ( even though it does not have any persistent state, which it could conceivably do

Re: RFR [9] : Add default[Read|Write]Object to java.util.Date

+1 On 3/20/2015 2:55 PM, Chris Hegarty wrote: Trivial change that came up a few months ago, and again recently, that java.util.Date should call default[Read|Write]Object to strictly conform to the Serialization spec ( even though it does not have any persistent state, which it could conceivab

RFR [9] : Add default[Read|Write]Object to java.util.Date

Trivial change that came up a few months ago, and again recently, that java.util.Date should call default[Read|Write]Object to strictly conform to the Serialization spec ( even though it does not have any persistent state, which it could conceivably do at some point in the future, but probably w

[9] RFR 8074840: Resolve disabled warnings for libjli and libjli_static

Please review the following change which fixes a number of native compilation warnings in files related to libjli. Bug: https://bugs.openjdk.java.net/browse/JDK-8074840 Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8074840/webrev.01/webrev/ I built the code across all the OracleJDK platf

[9] RFC on 8074467: In some cases, BigDecimal's toBigInteger() produces wrong result

Comments from the “numerologists” on this list would be appreciated. I investigated this issue https://bugs.openjdk.java.net/browse/JDK-8074467 running the supplied test on JDK 8u40 and 9dev on native Mac OS 10.9.5 and VirtualBox Windows 7 Pro VM for the “BLOCK” values 1000 and 1500, as specifi

Re: [9] RFR of 8075362: j.u.Properties.load() methods have misaligned @throws clauses

Hi Brian, Looks fine. Roger On 3/17/2015 3:15 PM, Brian Burkhalter wrote: Follow-on to correct some insufficiencies pointed out in http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-March/032289.html Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK

Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

On 20 Mar 2015, at 13:03, David M. Lloyd wrote: ... > An interesting aspect of this approach is that it deals with a problem in the > serialization spec [1] where it specifically says that serializable classes > should be reading/writing stream fields always, and before reading/writing > other

Re: Fw: Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

On 03/20/2015 09:31 AM, Peter Levart wrote: On 03/20/2015 02:03 PM, David M. Lloyd wrote: private static void altReadObject(ObjectInputStream in, FieldAccess fieldAccess) throws IOException, ClassNotFoundException { // the same as in readObject, but doesn't have direct access to ins

Re: RFR [9] 8071474: Better failure atomicity for default read object

Hi Peter, On 20 Mar 2015, at 14:06, Peter Levart wrote: > On 03/19/2015 10:26 PM, Chris Hegarty wrote: >> The current implementation of defaultReadObject sets non-primitive field >> values one at a time, without first checking that all their types are >> assignable. If, for example, the assign

Re: RFR 8071667 : HashMap.computeIfAbsent() adds entry that HashMap.get() does not find.

On 3/20/15 4:40 AM, Chris Hegarty wrote: Yes, the spec changes look good. Thanks. Brent, would you mind if I assign 8062841 [1] to you, so you can bring in the CHM implementation changes? Also maybe CHM could be added to the test? I've assigned it to myself. -Brent

Re: 8026049: (bf) Intrinsify ByteBuffer.put{Int, Double, Float, ...} methods

Webrev at http://cr.openjdk.java.net/~aph/unaligned.hotspot.7/ http://cr.openjdk.java.net/~aph/unaligned.jdk.7/ I've moved the test case into the hotspot repo The test case is diagnostic, not experimental Andrew.

Re: String.contains(CharSequence) calls toString on argument

> > If CharSequence is mutable and thread-safe, I would expect toString() > implementation to provide the atomic snapshot (e.g. do the > synchronization). Therefore, I find Sherman's argument interesting. That's my point about "it ought to be up to the caller" -- they're in a position to make thi

Re: String.contains(CharSequence) calls toString on argument

If CharSequence is mutable and thread-safe, I would expect toString() implementation to provide the atomic snapshot (e.g. do the synchronization). Therefore, I find Sherman's argument interesting. On the other hand, we don't seem to be having any details/requirements for contains(CharSequence) w.r

Re: String.contains(CharSequence) calls toString on argument

Yes, but that ought to be for the caller to decide. Also, although the resulting String is immutable, toString() itself may observe mutation. On Fri, Mar 20, 2015 at 11:40 AM, Xueming Shen wrote: > On 03/20/2015 02:34 AM, Tomasz Kowalczewski wrote: > >> Hello! >> >> Current implementation of St

Re: String.contains(CharSequence) calls toString on argument

On 03/20/2015 02:34 AM, Tomasz Kowalczewski wrote: Hello! Current implementation of String.contains that accepts CharSequence calls toString on it and passes resulting string to indexOf(String). This IMO defeats the purpose of using CharSequences (that is to have a mutable character buffer and n

Re: [8] RFR of 8066985: Java Webstart downloading packed files can result in Timezone set to UTC

On 3/19/2015 7:29 PM, Peter Levart wrote: On 03/19/2015 04:46 PM, Peter Levart wrote: On 03/19/2015 02:53 PM, Kumar Srinivasan wrote: Mikhail, You can move the common utilitieschangeDefaultTimeZoneToUtc and restoreDefaultTimeZoneto Utils.java. Also I am not sure how effective the test is ?

Re: Fw: Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

On 03/20/2015 02:03 PM, David M. Lloyd wrote: private static void altReadObject(ObjectInputStream in, FieldAccess fieldAccess) throws IOException, ClassNotFoundException { // the same as in readObject, but doesn't have direct access to instance state, so everything must go through

Re: RFR [9] 8071474: Better failure atomicity for default read object

On 03/19/2015 10:26 PM, Chris Hegarty wrote: The current implementation of defaultReadObject sets non-primitive field values one at a time, without first checking that all their types are assignable. If, for example, the assignment of the last non-primitive value fails, a CCE is thrown, and the

Re: Fw: Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

On 03/20/2015 05:49 AM, Peter Firmstone wrote: - Original message - From Peter Levart Sent Fri, 20 Mar 2015, 01:21:17 EST To Peter Firmstone , core-libs-dev@openjdk.java.net Subject Re: RFR [9] 8071472: Add field access to support setting final fields in readObject On 03/19/2015 11

Fw: Re: RFR [9] 8071472: Add field access to support setting final fields in readObject

- Original message - >From Peter Levart Sent  Fri, 20 Mar 2015, 01:21:17 EST To Peter Firmstone , core-libs-dev@openjdk.java.net Subject Re: RFR [9] 8071472: Add field access to support setting final fields in readObject > On 03/19/2015 11:35 AM, Peter Firmstone wrote: > > Chris / Peter

Re: String.contains(CharSequence) calls toString on argument

I agree. Adding a new method that doesn't toString the CS would be desirable. In these special cases, it's perfectly fine if there's no string intrinsic that kicks in (and thus the actual operation is a bit slower) -- the goal is to avoid the allocation. On Fri, Mar 20, 2015 at 8:41 AM, Tomasz K

Re: String.contains(CharSequence) calls toString on argument

It is more about custom implementations of CharSuqence that may be backed by some exotic matter and calling toString on them to create actual String goes against purpose of CharSequence in the first place (esp. when passing it to a method that accepts CharSequence). For JDK user it is easy to avoid

Re: String.contains(CharSequence) calls toString on argument

On 03/20/2015 03:28 PM, Aleksey Shipilev wrote: > I wonder if the change is "only" about specializing > indexOf(CharSequence) on the Java side to shortcut to indexOf(String) > and others, like String.contentEquals(CharSequence) already does. Ah, sorry for the confusion, contains(CharSequence) alre

Re: String.contains(CharSequence) calls toString on argument

Hi, On 03/20/2015 03:19 PM, Tomasz Kowalczewski wrote: > Thank you, that is the kind of feedback I need! Claes' feedback is spot on. While the change might help, be prepared that performance research in the area covered by compiler intrinsics is hard. > I have also asked myself this question. Ch

Re: String.contains(CharSequence) calls toString on argument

Thank you, that is the kind of feedback I need! I have also asked myself this question. Changing indexOf to accept CharSequence is the most intrusive move. Safest way is to just implement contains in a different way (taking code from indexOf) but adding indexOf(CharSequence) might provide more val

Re: RFR 8071667 : HashMap.computeIfAbsent() adds entry that HashMap.get() does not find.

On 20 Mar 2015, at 09:52, Paul Sandoz wrote: > > On Mar 19, 2015, at 11:31 PM, Brent Christian > wrote: > >> Hi, Paul >> >> Thank you for the suggested doc adjustments. They're applied here: >> http://cr.openjdk.java.net/~bchristi/8071667/webrev.3/ >> > > Looking good. Yes, the spec ch

Re: String.contains(CharSequence) calls toString on argument

Hi! While enabling use of CharSequence would seem desirable, there are some drawbacks that needs to be kept in mind: - String.indexOf(String) and friends are typically intrinsified on String in ways that might not port over well to other implementations of CharSequence - there might not even b

Re: RFR 8071667 : HashMap.computeIfAbsent() adds entry that HashMap.get() does not find.

On Mar 19, 2015, at 11:31 PM, Brent Christian wrote: > Hi, Paul > > Thank you for the suggested doc adjustments. They're applied here: > http://cr.openjdk.java.net/~bchristi/8071667/webrev.3/ > Looking good. Small point. We could clear up some inconsistency over using "the function" or t

String.contains(CharSequence) calls toString on argument

Hello! Current implementation of String.contains that accepts CharSequence calls toString on it and passes resulting string to indexOf(String). This IMO defeats the purpose of using CharSequences (that is to have a mutable character buffer and not allocate unnecessary objects). Is changing this a

Re: JDK 9 RFR of JDK-8075573: Add jdk_other and jdk_svc to jdk tier 2 test definition

On 20/03/2015 04:16, joe darcy wrote: Hello, Two test groups were inadvertently omitted from the definition of tier 2 tests: JDK-8075573: Add jdk_other and jdk_svc to jdk tier 2 test definition Please review the patch below to add them in. I thought it was deliberate to omit these but

Re: RFR 8075560 Typo in Javadoc for java.util.Optional.equals()

On 20/03/2015 08:48, Paul Sandoz wrote: Hi, The patch below fixes a silly typo in the JavaDoc of the Optional*.equals methods. Looks fine.

RFR 8075560 Typo in Javadoc for java.util.Optional.equals()

Hi, The patch below fixes a silly typo in the JavaDoc of the Optional*.equals methods. Paul. --- a/src/java.base/share/classes/java/util/Optional.java Mon Mar 16 17:44:34 2015 +0100 +++ b/src/java.base/share/classes/java/util/Optional.java Fri Mar 20 09:36:13 2015 +0100 @@ -346,7