insaf021 commented on code in PR #340:
URL: https://github.com/apache/commons-math/pull/340#discussion_r4396864640
##########
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:
Done, switched JACOBI_COEFFICIENTS to a ConcurrentHashMap and seed the list
inside computeIfAbsent. Lookups for existing keys no longer contend with a new
key being seeded. The HashMap import went with it.
##########
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:
Agreed, only the growth and the copy into the double[] need the list lock.
Moved the PolynomialFunction construction outside the synchronized block.
PolynomialsUtilsTest, including the concurrent test, still passes.
--
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]
