[PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics
Chris Dennis
chris.w.dennis at gmail.com
Thu Apr 6 14:42:42 UTC 2017
# 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 <jbluettduncan at 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.dennis at 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)?
>
>
More information about the core-libs-dev
mailing list