# HG changeset patch # User chris_dennis # Date 1491485015 14400 # Thu Apr 06 09:23:35 2017 -0400 # Node ID d789970b8393032457885e739d76919f714bbd50 # Parent c0aecf84349c97f4241eab01f7bbfb7660d51be1 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics
diff --git a/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java b/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java --- a/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java +++ b/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java @@ -76,6 +76,47 @@ public DoubleSummaryStatistics() { } /** + * Construct a non-empty instance with the supplied min, max, count, and sum. + * + * <p>If {@code count} is zero then the remaining parameters are ignored + * and an empty instance is constructed. + * <p>If the arguments are inconsistent then an {@code IllegalArgumentException} + * is thrown. The necessary consistent argument conditions are: + * <ul> + * <li>{@code count} &ge 0</li> + * <li>{@code min} &le {@code max}</li> + * <li>({@code count} × {@code max}) &ge {@code sum}</li> + * <li>({@code count} × {@code min}) &le {@code sum}</li> + * </ul> + * This enforcement of argument consistency means that the retrieved set of + * values from a {@code DoubleSummaryStatistics} instance may not be a legal + * set of arguments for this constructor due to arithmetic overflow in the + * original instance. + * + * @param min the minimum value + * @param max the maximum value + * @param count the count of values + * @param sum the sum of all values + * @throws IllegalArgumentException if the arguments are inconsistent + */ + public DoubleSummaryStatistics(double min, double max, long count, double sum) throws IllegalArgumentException { + if (count < 0L) { + throw new IllegalArgumentException("Negative count value"); + } else if (count > 0L) { + if (min > max) throw new IllegalArgumentException("Minimum greater than maximum"); + if (count * max < sum) throw new IllegalArgumentException("Maximum inconsistent with sum and count"); + if (count * min > sum) throw new IllegalArgumentException("Minimum inconsistent with sum and count"); + + this.count = count; + this.sum = sum; + this.simpleSum = sum; + this.sumCompensation = 0.0d; + this.min = min; + this.max = max; + } + } + + /** * Records another value into the summary information. * * @param value the input value diff --git a/src/java.base/share/classes/java/util/IntSummaryStatistics.java b/src/java.base/share/classes/java/util/IntSummaryStatistics.java --- a/src/java.base/share/classes/java/util/IntSummaryStatistics.java +++ b/src/java.base/share/classes/java/util/IntSummaryStatistics.java @@ -27,6 +27,9 @@ import java.util.function.IntConsumer; import java.util.stream.Collector; +import static java.lang.Math.multiplyExact; +import static java.lang.Math.multiplyFull; + /** * A state object for collecting statistics such as count, min, max, sum, and * average. @@ -76,6 +79,49 @@ public IntSummaryStatistics() { } /** + * Construct a non-empty instance with the supplied min, max, count, and sum. + * + * <p>If {@code count} is zero then the remaining parameters are ignored + * and an empty instance is constructed. + * <p>If the arguments are inconsistent then an {@code IllegalArgumentException} + * is thrown. The necessary consistent argument conditions are: + * <ul> + * <li>{@code count} &ge 0</li> + * <li>{@code min} &le {@code max}</li> + * <li>({@code count} × {@code max}) &ge {@code sum}</li> + * <li>({@code count} × {@code min}) &le {@code sum}</li> + * </ul> + * This enforcement of argument consistency means that the retrieved set of + * values from an {@code IntSummaryStatistics} instance may not be a legal + * set of arguments for this constructor due to arithmetic overflow in the + * original instance. + * + * @param min the minimum value + * @param max the maximum value + * @param count the count of values + * @param sum the sum of all values + * @throws IllegalArgumentException if the arguments are inconsistent + */ + public IntSummaryStatistics(int min, int max, long count, long sum) throws IllegalArgumentException { + if (count < 0L) { + throw new IllegalArgumentException("Negative count value"); + } else if (count > 0L) { + if (min > max) throw new IllegalArgumentException("Minimum greater than maximum"); + try { + if (multiplyExact(count, max) < sum) throw new IllegalArgumentException("Maximum inconsistent with sum and count"); + } catch (ArithmeticException e) {} + try { + if (multiplyExact(count, min) > sum) throw new IllegalArgumentException("Minimum inconsistent with sum and count"); + } catch (ArithmeticException e) {} + + this.count = count; + this.sum = sum; + this.min = min; + this.max = max; + } + } + + /** * Records a new value into the summary information * * @param value the input value diff --git a/src/java.base/share/classes/java/util/LongSummaryStatistics.java b/src/java.base/share/classes/java/util/LongSummaryStatistics.java --- a/src/java.base/share/classes/java/util/LongSummaryStatistics.java +++ b/src/java.base/share/classes/java/util/LongSummaryStatistics.java @@ -28,6 +28,8 @@ import java.util.function.LongConsumer; import java.util.stream.Collector; +import static java.lang.Math.multiplyExact; + /** * A state object for collecting statistics such as count, min, max, sum, and * average. @@ -77,6 +79,49 @@ public LongSummaryStatistics() { } /** + * Construct a non-empty instance with the supplied min, max, count, and sum. + * + * <p>If {@code count} is zero then the remaining parameters are ignored + * and an empty instance is constructed. + * <p>If the arguments are inconsistent then an {@code IllegalArgumentException} + * is thrown. The necessary consistent argument conditions are: + * <ul> + * <li>{@code count} &ge 0</li> + * <li>{@code min} &le {@code max}</li> + * <li>({@code count} × {@code max}) &ge {@code sum}</li> + * <li>({@code count} × {@code min}) &le {@code sum}</li> + * </ul> + * This enforcement of argument consistency means that the retrieved set of + * values from a {@code LongSummaryStatistics} instance may not be a legal + * set of arguments for this constructor due to arithmetic overflow in the + * original instance. + * + * @param min the minimum value + * @param max the maximum value + * @param count the count of values + * @param sum the sum of all values + * @throws IllegalArgumentException if the arguments are inconsistent + */ + public LongSummaryStatistics(long min, long max, long count, long sum) throws IllegalArgumentException { + if (count < 0L) { + throw new IllegalArgumentException("Negative count value"); + } else if (count > 0L) { + if (min > max) throw new IllegalArgumentException("Minimum greater than maximum"); + try { + if (multiplyExact(count, max) < sum) throw new IllegalArgumentException("Maximum inconsistent with sum and count"); + } catch (ArithmeticException e) {} + try { + if (multiplyExact(count, min) > sum) throw new IllegalArgumentException("Minimum inconsistent with sum and count"); + } catch (ArithmeticException e) {} + + this.count = count; + this.sum = sum; + this.min = min; + this.max = max; + } + } + + /** * Records a new {@code int} value into the summary information. * * @param value the input value > On Apr 6, 2017, at 10:32 AM, Jonathan Bluett-Duncan <jbluettdun...@gmail.com> > wrote: > > Hi Chris, > > Unfortunately the patch you sent (in what I presume was an attachment) is > missing. I believe the OpenJDK mailing list servers intentionally strip out > attachments in all emails, which seems to be at odds with the advice given in > http://openjdk.java.net/contribute/. (Either the contribution advice or the > servers should be changed, ideally!) > > I understand that one can host patches somewhere official, but I've forgotten > the details of the process. > > Can anyone help? > > Jonathan > > > On 6 April 2017 at 15:08, Chris Dennis <chris.w.den...@gmail.com> wrote: > Hi Paul (et al) > > Like all things API there are wrinkles here when it comes to implementing. > > This patch isn’t final, there appears to be no existing test coverage for > these classes beyond testing the compensating summation used in the double > implementation, and I left off adding any until it was decided how much > parameter validation we want (since that’s the only testing that can really > occur here). > > The wrinkles relate to the issues around instances that have suffered > overflow in count and/or sum which the existing implementation doesn’t defend > against: > > * I chose to ignore all parameters if 'count == 0’ and just leave the > instance empty. I held off from validating min, max and count however. This > obviously 'breaks things’ (beyond how broken they would already be) if count > == 0 only due to overflow. > * I chose to fail if count is negative. > * I chose to enforce that max and min are logically consistent with count > and sum, this can break the moment either one of the overflows as well (I’m > also pretty sure that the FPU logic is going to suffer here too - there might > be some magic I can do with a pessimistic bit of rounding on the FPU > multiplication result). > > I intentionally went with the most aggressive parameter validation here to > establish one end of what could be implemented. There are arguments for this > and also arguments for the other extreme (no validation at all). Personally > I’m in favor of the current version where the creation will fail if the > inputs are only possible through overflow (modulo fixing the FPU precision > issues above) even though it’s at odds with approach of the existing > implementation. > > Would appreciate thoughts/feedback. Thanks. > > Chris > > > P.S. I presume tests would be required for the parameter checking (once it is > finalized)? > >