Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (nike.apache.org: local policy)
Date: Sat, 21 Jul 2012 15:08:34 +0200
From: Gilles Sadowski <gilles@harfang.homelinux.org>
To: dev@commons.apache.org
Subject: Re: [Math] Synchronization
Message-ID: <20120721130834.GM25337@dusk.harfang.homelinux.org>
Mail-Followup-To: dev@commons.apache.org
References: <20120721000818.EE206238889B@eris.apache.org>
 <20120721003908.GK25337@dusk.harfang.homelinux.org>
 <500A1373.7020501@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <500A1373.7020501@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-DCC-scarlet.be-Metrics: eir 20002; Body=1 Fuz1=1 Fuz2=1
X-Virus-Scanned: clamav-milter 0.97.1-exp at eir
X-Virus-Status: Clean

On Fri, Jul 20, 2012 at 07:26:59PM -0700, Phil Steitz wrote:
> 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.

Indeed, I needed some "sleep". :-)
Test added in revision 1364075.

> 
> Note that there is a performance price for the synchronization,
> which may outweigh the benefit of adding it.

I thought of adding that because one might want to parallelize the
integration computations while the quadrature rules factory is shared.
[See e.g. MATH-827, line 41 in the posted code.]
Without "synchronized", all threads will compute all the rules.

On my machine (4 cores), in the absence of synchronization, the above test
indicated that the "computeRule" method was called 18 times!


Regards,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org