On 7/20/12 5:39 PM, Gilles Sadowski wrote: > Hi. > >> Author: erans >> Date: Sat Jul 21 00:08:18 2012 >> New Revision: 1364024 >> >> URL: http://svn.apache.org/viewvc?rev=1364024&view=rev >> Log: >> MATH-797 >> Performance: synchronization should ensure that the computation of each >> rule will be performed once, even if the factory is accessed from multiple >> threads. >> >> Modified: >> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/analysis/integration/gauss/BaseRuleFactory.java >> >> Modified: >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/analysis/integration/gauss/BaseRuleFactory.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/analysis/integration/gauss/BaseRuleFactory.java?rev=1364024&r1=1364023&r2=1364024&view=diff >> ============================================================================== >> --- >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/analysis/integration/gauss/BaseRuleFactory.java >> (original) >> +++ >> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/analysis/integration/gauss/BaseRuleFactory.java >> Sat Jul 21 00:08:18 2012 >> @@ -68,13 +68,14 @@ public abstract class BaseRuleFactory<T >> >> /** >> * Gets a rule. >> - * Rules are computed once, and cached. >> + * Synchronization ensures that rules will be computed and added to the >> + * cache at most once. >> * The returned rule is a reference into the cache. >> * >> * @param numberOfPoints Order of the rule to be retrieved. >> * @return the points and weights corresponding to the given order. >> */ >> - protected Pair<T[], T[]> getRuleInternal(int numberOfPoints) { >> + protected synchronized Pair<T[], T[]> getRuleInternal(int >> numberOfPoints) { >> final Pair<T[], T[]> rule = pointsAndWeights.get(numberOfPoints); >> if (rule == null) { >> addRule(computeRule(numberOfPoints)); > Any idea about how to set up a unit test that proves the claim?
Have a look at the unit tests in [pool]. There are lots of examples there setting up race conditions. IIUC what is going on here, you could set up a unit test that failed before this change and succeeded after by creating a rule factory that has latency in its addRule or computeRule method (add a sleep in the body somewhere). Modify its addRule to track the number of times it is called. Then create the race by launching two threads with no delay between that both invoke getRuleInternal. Before adding the synch, the addRule counter will be 2 (assuming there is enough latency in it or computeRule to ensure the first thread does not complete before the second enters the block). After this patch, the counter should be 1. Note that there is a performance price for the synchronization, which may outweigh the benefit of adding it. Phil > > > Thanks, > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
