aherbert commented on code in PR #340:
URL: https://github.com/apache/commons-math/pull/340#discussion_r4386592021
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java:
##########
@@ -228,24 +228,26 @@ public static PolynomialFunction
createJacobiPolynomial(final int degree, final
// select the appropriate list
final JacobiKey key = new JacobiKey(v, w);
- if (!JACOBI_COEFFICIENTS.containsKey(key)) {
- synchronized(JACOBI_COEFFICIENTS) {
- if (!JACOBI_COEFFICIENTS.containsKey(key)) {
- // allocate a new list for v, w
- final List<BigFraction> list = new ArrayList<>();
- JACOBI_COEFFICIENTS.put(key, list);
-
- // Pv,w,0(x) = 1;
- list.add(BigFraction.ONE);
-
- // P1(x) = (v - w) / 2 + (2 + v + w) * X / 2
- list.add(BigFraction.of(v - w, 2));
- list.add(BigFraction.of(2 + v + w, 2));
- }
+ final List<BigFraction> coefficients;
Review Comment:
Since this is using a `Map` this is more explicit in its support of
concurrency using a `ConcurrentHashMap`:
```java
JACOBI_COEFFICIENTS = new ConcurrentHashMap<>();
// ...
List<BigFraction> coefficients =
JACOBI_COEFFICIENTS.computeIfAbsent(key, k -> {
// allocate a new list for v, w
final List<BigFraction> list = new ArrayList<>();
// Pv,w,0(x) = 1;
list.add(BigFraction.ONE);
// P1(x) = (v - w) / 2 + (2 + v + w) * X / 2
list.add(BigFraction.of(v - w, 2));
list.add(BigFraction.of(2 + v + w, 2));
return list;
});
```
Using a `ConcurrentHashMap` will not lock the entire table allowing other
concurrent threads to obtain coefficients that exist.
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/polynomials/PolynomialsUtils.java:
##########
@@ -373,25 +375,25 @@ private static PolynomialFunction buildPolynomial(final
int degree,
if (degree > maxDegree) {
computeUpToDegree(degree, maxDegree, generator, coefficients);
}
- }
- // coefficient for polynomial 0 is l [0]
- // coefficients for polynomial 1 are l [1] ... l [2] (degrees 0 ... 1)
- // coefficients for polynomial 2 are l [3] ... l [5] (degrees 0 ... 2)
- // coefficients for polynomial 3 are l [6] ... l [9] (degrees 0 ... 3)
- // coefficients for polynomial 4 are l[10] ... l[14] (degrees 0 ... 4)
- // coefficients for polynomial 5 are l[15] ... l[20] (degrees 0 ... 5)
- // coefficients for polynomial 6 are l[21] ... l[27] (degrees 0 ... 6)
- // ...
- final int start = degree * (degree + 1) / 2;
-
- final double[] a = new double[degree + 1];
- for (int i = 0; i <= degree; ++i) {
- a[i] = coefficients.get(start + i).doubleValue();
- }
+ // coefficient for polynomial 0 is l [0]
+ // coefficients for polynomial 1 are l [1] ... l [2] (degrees 0
... 1)
+ // coefficients for polynomial 2 are l [3] ... l [5] (degrees 0
... 2)
+ // coefficients for polynomial 3 are l [6] ... l [9] (degrees 0
... 3)
+ // coefficients for polynomial 4 are l[10] ... l[14] (degrees 0
... 4)
+ // coefficients for polynomial 5 are l[15] ... l[20] (degrees 0
... 5)
+ // coefficients for polynomial 6 are l[21] ... l[27] (degrees 0
... 6)
+ // ...
+ final int start = degree * (degree + 1) / 2;
+
+ final double[] a = new double[degree + 1];
+ for (int i = 0; i <= degree; ++i) {
+ a[i] = coefficients.get(start + i).doubleValue();
+ }
- // build the polynomial
- return new PolynomialFunction(a);
+ // build the polynomial
+ return new PolynomialFunction(a);
Review Comment:
building the polynomial does not have to be synchronized once the
coefficients to the required degreee are created:
```java
final double[] a;
synchronized (coefficients) {
// ...
}
// build the polynomial
return new PolynomialFunction(a);
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
