Re: Fwd: JDK 9 RFR of JDK-8030942: Explicitly state floating-point summation requirements on non-finite inputs

2014-07-21 Thread Joe Darcy

On 07/18/2014 12:00 PM, Georgiy Rakov wrote:


On 18.07.2014 20:14, Joe Darcy wrote:

Hello Georgiy,

On 07/18/2014 05:29 AM, Georgiy Rakov wrote:

Hello Joe,

could you please clarify by short example following assertion:

  154  * If the exact sum is infinite, a properly-signed infinity is
  155  * returned.

I'm afraid I don't quite understand what you mean here by 'exact sum'.


By "exact sum," the sum absent any floating-point rounding, the sum 
you would get using infinite precision to operate on the values in 
question.


The sentence in question is intended to be a short way of saying "If 
you have same-signed infinities in your input, the result will be an 
infinity of that sign." In particular, this disallows the behavior 
that was fixed before JDK 8 GA where having infinities in the input 
would cause a NaN to be returned because of how the compensated 
summation code manipulated those values.



Thanks, I see,
however it seems to me a bit confusing, since the term "infinite exact 
sum" seems to me not obvious and I believe it needs some definition. 
I'd like to suggest to use more straightforward approach, that is as 
you've said: "If you have same-signed infinities in your input, the 
result will be an infinity of that sign.". I believe it would be more 
clear for end user (at least for me :)) and from conformance point of 
view.


Besides it seems to me a bit questionable. For instance "inexact some" 
looks like more appropriate, since overflowing to infinity occurs when 
_actual _sum exceeds the limit. By actual sum I mean sum resulted from 
actual summation with all the rounding happened. There wouldn't be 
such questions, provided straightforward approach is used.


Thanks,
Georgiy.



In response to previous feedback, I propose this revised change to the 
specification:


--- a/src/share/classes/java/util/DoubleSummaryStatistics.java Sat Jul 
19 11:22:08 2014 +0800
+++ b/src/share/classes/java/util/DoubleSummaryStatistics.java Mon Jul 
21 18:02:54 2014 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2012, 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
@@ -129,9 +129,6 @@
  * Returns the sum of values recorded, or zero if no values have been
  * recorded.
  *
- * If any recorded value is a NaN or the sum is at any point a NaN
- * then the sum will be NaN.
- *
  *  The value of a floating-point sum is a function both of the
  * input values as well as the order of addition operations. The
  * order of addition operations of this method is intentionally
@@ -143,6 +140,44 @@
  * numerical sum compared to a simple summation of {@code double}
  * values.
  *
+ * Because of the unspecified order of operations and the
+ * possibility of using differing summation schemes, the output of
+ * this method may vary on the same input values.
+ *
+ * Various conditions can result in a non-finite sum being
+ * computed. This can occur even if the all the recorded values
+ * being summed are finite. If any recorded value is non-finite,
+ * the sum will be non-finite:
+ *
+ * 
+ *
+ * If any recorded value is a NaN, then the final sum will be
+ * NaN.
+ *
+ * If the recorded values contain one or more infinities, the
+ * sum will be infinite or NaN.
+ *
+ * 
+ *
+ * If the recorded values contain infinities of opposite sign,
+ * the sum will be NaN.
+ *
+ * If the recorded values contain infinities of one sign and
+ * an intermediate sum overflows to an infinity of the opposite
+ * sign, the sum may be NaN.
+ *
+ * 
+ *
+ * 
+ *
+ * It is possible for intermediate sums of finite values to
+ * overflow into opposite-signed infinities; if that occurs, the
+ * final sum will be NaN even if the recorded values are all
+ * finite.
+ *
+ * If all the recorded values are zero, the sign of zero is
+ * not guaranteed to be preserved in the final sum.
+ *
  * @apiNote Values sorted by increasing absolute magnitude tend to 
yield

  * more accurate results.
  *
@@ -193,15 +228,9 @@
  * Returns the arithmetic mean of values recorded, or zero if no
  * values have been recorded.
  *
- * If any recorded value is a NaN or the sum is at any point a NaN
- * then the average will be code NaN.
- *
- * The average returned can vary depending upon the order in
- * which values are recorded.
- *
- * This method may be implemented using compensated summation or
- * other technique to reduce the error bound in the {@link #getSum
- * numerical sum} used to compute the average.
+ *  The computed average can vary numerically and have th

Re: RFR [8051382] Optimize java.lang.reflect.Modifier.toString()

2014-07-21 Thread Martin Buchholz
On Mon, Jul 21, 2014 at 10:58 AM, Joe Darcy  wrote:

> Hello,
>
> As a general comment, I think we should use more of StringJoiner in the
> JDK libraries; it would help get rid of some awkward loops, even if it
> isn't that compelling a code benefit in this case.
>
>
In performance critical contexts, one can almost always do a little better
than to use StringJoiner.  StringJoiner feels like an odd API.  For
performance critical uses, there is no way to presize the output.  As a
community, we have still not learned that use of constructors (instead of
factory methods) is usually a design mistake.


> While performance is an important concern, I don't know if producing
> modifier strings is actually performance critical.
>
>
I agree that Modifier.toString is probably not performance critical.


Re: Map.compute() confusing Javadoc

2014-07-21 Thread Mike Duigou
Hi Roman;

Somewhat unfortunately, "just return null" is what the default and all 
conforming implementations do of compute do when presented with a Map 
containing a mapping to null and a mapping function returning null. The 
specification of the new Java 8 Map methods does not always handle maps 
containing null values as clean as might be wished. This is mostly to maintain 
consistent semantics with some pre-java 8 ConcurrentHashMap operations. The 
chain of debatable decisions all starts with "putIfAbsent()" and then tries to 
proceed consistently to the other new default operations. In general a 
null-value mapping in a Map is treated as absent--the same way as CHM would 
treat a get() returning null. Every attempt I made at improving the behaviour 
of mapping to null values ended up being weirder and more mysterious than what 
Java 8 shipped with.

Sorry.

Mike

On Jul 20 2014, at 15:25 , David Holmes  wrote:

> See:
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018251.html
> 
> and related bug report.
> 
> David
> 
> On 21/07/2014 6:01 AM, Paul Sandoz wrote:
>> Begin forwarded message:
>>>  Пересылаемое сообщение
>>> 17.07.2014, 19:20, "Roman Leventov" :
>>> 
>>> Map.compute() Javadoc has the following paragraph:
>>> 
>>> The default implementation is equivalent to performing the following steps 
>>> for this map, then returning the current value or null if absent:
>>> 
>>>  V oldValue = map.get(key);
>>>  V newValue = remappingFunction.apply(key, oldValue);
>>>  if (oldValue != null ) {
>>> if (newValue != null)
>>>map.put(key, newValue);
>>> else
>>>map.remove(key);
>>>  } else {
>>> if (newValue != null)
>>>map.put(key, newValue);
>>> else
>>>return null;
>>>  }
>>> 
>>> 
>>> But this code don't correspond neither verbal description of the behaviour 
>>> nor the actual default implementation in java.util.Map. If the oldValue is 
>>> null and newValue is null, map should still remove the key, not to just 
>>> `return null;`
>>> 
>> 



Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Claes Redestad


On 2014-07-21 22:05, Peter Levart wrote:


On 07/21/2014 09:21 PM, Claes Redestad wrote:

Hi,

 was asked offline to add a length check at the start (prevents 
potentially costly scans for huge, invalid input; negligible 
performance impact for normal cases) and realized dash1 < 0 || dash2 
< 0 || dash3 < 0 implies dash4 < 0, so the first three checks are 
unnecessary and can be skipped:


http://cr.openjdk.java.net/~redestad/8006627/webrev.5


Hi Claes,

dash1 < 0 || dash2 < 0 || dash3 < 0 does not imply dash4 < 0

Take for example the following input: "0-0-0"

dash1 = 1
dash2 = 3
dash3 = -1
dash4 = 1 (again)

Well, this is embarrassing...


but the following check:

if (dash4 < 0 || name.indexOf('-', dash4 + 1) > 0)

is true in this case. It seems either dash4 < 0 or dash5 > 0 for any 
number of dashes in the string except exactly 4. It's just that this 
is not immediately evident for the casual reader. I recommend adding a 
comment for posterity.


... but that explains why all test cases worked and reinforced the 
erroneous assumption. Skipping the checks bring back a few percent in my 
microbenchmarks, so I guess adding an explanation as to why this 
actually works in context in an inline comment is the least we should do 
here:


+// For any valid input, dash1 through dash4 will be positive and dash5
+// negative, but it's enough to check dash4 and dash5:
+// - if dash1 is -1, dash4 will be -1
+// - if dash1 is positive but dash2 is -1, dash4 will be -1
+// - if dash1 and dash2 is positive, dash3 will be -1, dash4 will be
+//   positive, but so will dash5

http://cr.openjdk.java.net/~redestad/8006627/webrev.6

Thanks!

/Claes



Regards, Peter



 /Claes

On 2014-07-21 20:32, Claes Redestad wrote:

Hi,

 new webrev which ensures we always throw some kind of IAE for 
invalid inputs and adds a few tests to cover this behavior: 
http://cr.openjdk.java.net/~redestad/8006627/webrev.4


/Claes

On 2014-07-21 20:05, Claes Redestad wrote:

Hi,

 IIOB is invalid to throw here, so I'll fix that. 
NumberFormatException is a IllegalArgumentException, so I think 
it's a gray area if the dash4 + 1 < name.length()-check is needed.


 I sincerely hope keeping error messages as-is isn't required, though.

/Claes

On 2014-07-21 18:51, Peter Levart wrote:

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following 
inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before 
patch)

"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 
0-0-0-0- (before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException 
as before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before 
patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" 
(after patch)


"0-0-0--0": NumberFormatException: For input string: "" (before 
patch)

"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are 
all > 0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no 
less clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with 
tiered.


/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and 
improved slightly, primarily simplifying and speeding up the 
toString method slightly more:


http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, 
originally proposed by Steven Schlansker, rebased to use the 
allocation-free methods added in 
https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes






















Re: please review draft JEP: Convenience Factory Methods for Collections

2014-07-21 Thread Stuart Marks



On 7/19/14 2:04 AM, Remi Forax wrote:

You can combine these 2 approachesi using a Ruby like builder approach with a
lambda,
which provide a builder object (so static method call are replaced by instance
method call) avoiding the creation of too many entry objects
Map.of(b -> b
 .entry(k0, v0)
 .entry(k1, v1)
   ...
 .entry(kN, vN))


Very interesting! I'll take a look at this approach.

Some of the internal feedback on this JEP draft before I posted it here was, 
(jokingly) "What, it doesn't use lambda??" Now we might have a way to work 
lambda into this. :-)


s'marks



Re: please review draft JEP: Convenience Factory Methods for Collections

2014-07-21 Thread Stuart Marks

On 7/18/14 4:41 PM, David M. Lloyd wrote:

On 07/18/2014 06:36 PM, Stuart Marks wrote:

 Map.of()
.add(k0, v0)
.add(k1, v1)
  ...
.add(kN, vN);

this would result in O(N^2) performance and space allocation, though
most of the allocated space is garbage.


Adding another alternative, instead of producing intermediate maps which are
copied at each step for the sake of maintaining O(1) lookup, this approach could
instead create a linked-list-of-pairs which would be O(n) for lookups, but also
O(n) for space - you could then efficiently feed that to a "real" map
constructor, especially if each "map" also implemented Map.Entry, which would
make an iterator particularly lightweight, making the overall cost for, say, a
HashMap be O(n).

Not sure it's the "right" approach, just throwing it out there as a random
Friday thought.


... now being analyzed in the cold, harsh light of a Monday morning. Er, 
afternoon. :-)


The main difficulty here is if the type returned by add() is a Map, it has to 
obey all the requirements of the Map contract. That means that somebody has to 
enforce the uniqueness constraint of the keys. This will affect things like 
iterating over the map or asking for its size, and so forth. Either the extra 
work is done at addition time or later when certain results require respecting 
uniqueness.


If add() returns something other than a Map, such as a temporary data structure 
later used to construct a Map, then pushing the items onto a linked list is 
quite a sensible idea. (Maybe a linked list of arrays, to reduce per-element 
overhead.) We don't care if the keys are unique when they're being added, since 
that'll be taken care of later at map construction time. But this is essentially 
a builder, not incremental additions to a Map.


s'marks


Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Peter Levart


On 07/21/2014 09:21 PM, Claes Redestad wrote:

Hi,

 was asked offline to add a length check at the start (prevents 
potentially costly scans for huge, invalid input; negligible 
performance impact for normal cases) and realized dash1 < 0 || dash2 < 
0 || dash3 < 0 implies dash4 < 0, so the first three checks are 
unnecessary and can be skipped:


 http://cr.openjdk.java.net/~redestad/8006627/webrev.5


Hi Claes,

dash1 < 0 || dash2 < 0 || dash3 < 0 does not imply dash4 < 0

Take for example the following input: "0-0-0"

dash1 = 1
dash2 = 3
dash3 = -1
dash4 = 1 (again)

but the following check:

if (dash4 < 0 || name.indexOf('-', dash4 + 1) > 0)

is true in this case. It seems either dash4 < 0 or dash5 > 0 for any 
number of dashes in the string except exactly 4. It's just that this is 
not immediately evident for the casual reader. I recommend adding a 
comment for posterity.


Regards, Peter



 /Claes

On 2014-07-21 20:32, Claes Redestad wrote:

Hi,

 new webrev which ensures we always throw some kind of IAE for 
invalid inputs and adds a few tests to cover this behavior: 
http://cr.openjdk.java.net/~redestad/8006627/webrev.4


/Claes

On 2014-07-21 20:05, Claes Redestad wrote:

Hi,

 IIOB is invalid to throw here, so I'll fix that. 
NumberFormatException is a IllegalArgumentException, so I think it's 
a gray area if the dash4 + 1 < name.length()-check is needed.


 I sincerely hope keeping error messages as-is isn't required, though.

/Claes

On 2014-07-21 18:51, Peter Levart wrote:

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following 
inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before patch)
"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- 
(before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException as 
before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before 
patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after 
patch)


"0-0-0--0": NumberFormatException: For input string: "" (before patch)
"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are 
all > 0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with 
tiered.


/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and 
improved slightly, primarily simplifying and speeding up the 
toString method slightly more:


http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, 
originally proposed by Steven Schlansker, rebased to use the 
allocation-free methods added in 
https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes




















Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Claes Redestad

Hi,

 was asked offline to add a length check at the start (prevents 
potentially costly scans for huge, invalid input; negligible performance 
impact for normal cases) and realized dash1 < 0 || dash2 < 0 || dash3 < 
0 implies dash4 < 0, so the first three checks are unnecessary and can 
be skipped:


 http://cr.openjdk.java.net/~redestad/8006627/webrev.5

 /Claes

On 2014-07-21 20:32, Claes Redestad wrote:

Hi,

 new webrev which ensures we always throw some kind of IAE for invalid 
inputs and adds a few tests to cover this behavior: 
http://cr.openjdk.java.net/~redestad/8006627/webrev.4


/Claes

On 2014-07-21 20:05, Claes Redestad wrote:

Hi,

 IIOB is invalid to throw here, so I'll fix that. 
NumberFormatException is a IllegalArgumentException, so I think it's 
a gray area if the dash4 + 1 < name.length()-check is needed.


 I sincerely hope keeping error messages as-is isn't required, though.

/Claes

On 2014-07-21 18:51, Peter Levart wrote:

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following 
inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before patch)
"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- 
(before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException as 
before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before 
patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after 
patch)


"0-0-0--0": NumberFormatException: For input string: "" (before patch)
"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are 
all > 0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with 
tiered.


/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved 
slightly, primarily simplifying and speeding up the toString 
method slightly more:


http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally 
proposed by Steven Schlansker, rebased to use the 
allocation-free methods added in 
https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes


















Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Claes Redestad

Hi,

 new webrev which ensures we always throw some kind of IAE for invalid 
inputs and adds a few tests to cover this behavior: 
http://cr.openjdk.java.net/~redestad/8006627/webrev.4


/Claes

On 2014-07-21 20:05, Claes Redestad wrote:

Hi,

 IIOB is invalid to throw here, so I'll fix that. 
NumberFormatException is a IllegalArgumentException, so I think it's a 
gray area if the dash4 + 1 < name.length()-check is needed.


 I sincerely hope keeping error messages as-is isn't required, though.

/Claes

On 2014-07-21 18:51, Peter Levart wrote:

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before patch)
"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- 
(before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException as 
before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after 
patch)


"0-0-0--0": NumberFormatException: For input string: "" (before patch)
"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are 
all > 0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with 
tiered.


/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved 
slightly, primarily simplifying and speeding up the toString 
method slightly more:


http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally 
proposed by Steven Schlansker, rebased to use the allocation-free 
methods added in https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes
















Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Claes Redestad

Hi,

 IIOB is invalid to throw here, so I'll fix that. NumberFormatException 
is a IllegalArgumentException, so I think it's a gray area if the dash4 
+ 1 < name.length()-check is needed.


 I sincerely hope keeping error messages as-is isn't required, though.

/Claes

On 2014-07-21 18:51, Peter Levart wrote:

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before patch)
"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- 
(before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException as 
before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch)

"0-0-0--0": NumberFormatException: For input string: "" (before patch)
"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all 
> 0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with 
tiered.


/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved 
slightly, primarily simplifying and speeding up the toString method 
slightly more:


http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally 
proposed by Steven Schlansker, rebased to use the allocation-free 
methods added in https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes














Re: RFR [8051382] Optimize java.lang.reflect.Modifier.toString()

2014-07-21 Thread Joe Darcy

Hello,

As a general comment, I think we should use more of StringJoiner in the 
JDK libraries; it would help get rid of some awkward loops, even if it 
isn't that compelling a code benefit in this case.


While performance is an important concern, I don't know if producing 
modifier strings is actually performance critical.


Cheers,

-Joe

On 07/19/2014 08:58 AM, Martin Buchholz wrote:

StringJoiner seems written in a style suitable for an application, not in a
low-level performance-oriented style suitable for a JDK core library.  But
we can fix that.

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/StringJoiner-optimization/


On Fri, Jul 18, 2014 at 6:18 PM, Ivan Gerasimov 
wrote:


On 19.07.2014 3:07, Martin Buchholz wrote:


I took a quick look at StringJoiner.  It looks to me like this won't be
an optimization, because StringJoiner uses StringBuilder internally, and
will actually perform more total operations.


Unfortunately this is true.
Microbenchmarking shows that StringJoiner makes the things 30% slower,
which is sad.

Then I propose another simple patch giving +15% to the speed:

http://cr.openjdk.java.net/~igerasim/8051382/1/webrev/

Sincerely yours,
Inan






Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Xueming Shen


It appears input like "" now triggers different exception, the spec says
it should throw IAE.

-Sherman

On 07/21/2014 04:41 AM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with tiered.

/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved slightly, 
primarily simplifying and speeding up the toString method slightly more:

 http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally proposed by 
Steven Schlansker, rebased to use the allocation-free methods added in 
https://bugs.openjdk.java.net/browse/JDK-8041972

Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes












Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Peter Levart

Hi Claes,

Invalid inputs to UUID.fromString() behave a little different than 
before. IllegalArgumentException is not thrown for the following inputs:


For example:

"0": IllegalArgumentException: Invalid UUID string: 0 (before patch)
"0": IndexOutOfBoundsException (after patch)

"-0": IllegalArgumentException: Invalid UUID string: -0 (before patch)
"-0": NumberFormatException (after patch)

"0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- 
(before patch)

"0-0-0-0-": NumberFormatException (after patch)

The following input (and similar) do throw NumberFormatException as 
before, but messages are a little different. That's OK, I suppose.


"0-0-0-x-0": NumberFormatException: For input string: "x" (before patch)
"0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch)

"0-0-0--0": NumberFormatException: For input string: "" (before patch)
"0-0-0--0": NumberFormatException: (after patch)


The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 
0 and that dash4 + 1 < name.length()



Regards, Peter


On 07/21/2014 01:41 PM, Claes Redestad wrote:

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with tiered.

/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved 
slightly, primarily simplifying and speeding up the toString method 
slightly more:


 http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally 
proposed by Steven Schlansker, rebased to use the allocation-free 
methods added in https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes












Re: RFR : JDK-8046545 launcher fix to check function return values

2014-07-21 Thread Neil Toda


Hi Kumar

Actually, the null check macros have different parameters. 
NCRV_return_value is an integer.
RETURNVALUE in CHECK_JNI_RETURN is a macro,  which allows us to have 
only the two macros:

CHECK_JNI_RETURN
and
CHECK_JNI_RETRUN_EXCEPTION

I also think it is cleaner since there are only two, and they are for 
JNI, to keep them self contained.


Would someone be willing to review webrev-02, which contain Kumar's 
suggested change in the

comments included with the macros.

Thanks

-neil


On 7/19/2014 8:02 AM, Kumar Srinivasan wrote:
[ Since I am sponsoring this patch, I think jcheck needs one more 
Reviewer besides myself]


Neil,

looking at your webrev: 
http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/


Can we not re-use the existing macro for null check ?

#define NULL_CHECK_RETURN_VALUE(NCRV_check_pointer, NCRV_return_value)

so thus your new macro would become

+#define CHECK_JNI_RETURN(JNIRETURN, RETURNVALUE) \
+CHECK_JNI_RETURN_EXCEPTION(RETURNVALUE); \
-do { \
-if ((JNIRETURN) == NULL) { \
-JLI_ReportErrorMessage(JNI_ERROR); \
-RETURNVALUE; \
-} \
-} while (JNI_FALSE)
+NULL_CHECK_RETURN_VALUE(JNI_RETURN, RETURN_VALUE);


Kumar

On 7/18/2014 10:40 AM, Neil Toda wrote:


Thanks Kumar.  Yes, misspoke here.  Will correct the comment.

On 7/18/2014 10:35 AM, Kumar Srinivasan wrote:

Neil,
The fix looks good. However there is an inaccuracy in the comment:

+ *  Normally, JNI calls do not return if an exception is thrown.
+ *  However, this behavior can change in the future,
+ *  so check for thrown exceptions.

This is not true, JNI calls *will* return if an exception is thrown, 
however best
JNI practices dictate that a pending Exception(s) must be cleared or 
caught, before
attempting another JNI call. Under such circumstances the return 
value will usually
be an error or a null value. I suggest making this change to reflect 
this.


Thanks
Kumar



On 7/18/2014 9:53 AM, Neil Toda wrote:


Please review this launcher change.

Issue: https://bugs.openjdk.java.net/browse/JDK-8046545
webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-01/

Summary:

Introduce a set of macros for launcher to be used to check for 
certain conditions after

return from select functions.












Re: State of Serialization

2014-07-21 Thread Tom Hawtin

On 20/07/2014 11:57, Peter Firmstone wrote:


Since private methods are only be called by the ObjectOutputStream /
ObjectInputStream, during de-serialisation, subclass are not responsible
for calling these methods, hence subclass ProtectionDomain's are not
present in the Thread's AccessControlContext and as such are missing
from security checks, this is why it's currently essential for classes
to ensure that de-serialisation isn't performed in a privileged context.


It's more complicated than that. Even final serialisable classes may 
have security checks.



To improve security, it would be preferable to use a deserialization
constructor, required to be called by subclasses in the class
hierarchies, placing their ProtectionDomains in the stack context,
avoiding a number of security issues. Another benefit is the ability to
use final fields, while checking invariants during construction.


Certainly it would be better have a mechanism that better fitted in with 
non-serialisation mechanisms. Addressing this without unraveling too 
much when pulling on a thread, and without increasing complexity of 
corner cases, is non-trivial.


Tom


Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations

2014-07-21 Thread Claes Redestad

On 07/19/2014 02:59 PM, Ivan Gerasimov wrote:

This looks just beautiful!


Thanks!


But why do you need the digits() function at all?
In my opinion, using formatUnsignedLong directly would be no less 
clearer.


Sure!

http://cr.openjdk.java.net/~redestad/8006627/webrev.2/

Small improvement with client compiler; no measurable change with tiered.

/Claes



Sincerely yours,
Ivan

On 19.07.2014 8:59, Claes Redestad wrote:

Hi,

 after recent changes, this patch has been revisited and improved 
slightly, primarily simplifying and speeding up the toString method 
slightly more:


 http://cr.openjdk.java.net/~redestad/8006627/webrev.1/

 /Claes

On 2014-06-15 00:41, Claes Redestad wrote:

Hi,

please review this patch to improve UUID performance, originally 
proposed by Steven Schlansker, rebased to use the allocation-free 
methods added in https://bugs.openjdk.java.net/browse/JDK-8041972


Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/
Bug: https://bugs.openjdk.java.net/browse/JDK-8006627

Thanks!

/Claes