Trouble with commons-math

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Trouble with commons-math

David J. M. Karlsen
Hi folks!

I use DescriptiveStatistics - but it seems to fail under heavy usage.

The scenario:

I use commons math to track average response time in a spring interceptor.
The DescriptiveStatistics instance is held within an object instance.
All calls to this object is synchronized - I've triple-checked that.

Some times when I call

descriptiveStatistics.getMean()

it throws:

java.lang.IllegalArgumentException: begin + length > values.length
  at
org.apache.commons.math.stat.descriptive.AbstractUnivariateStatistic.test(AbstractUnivariateStatistic.java:89)
  at
org.apache.commons.math.stat.descriptive.moment.Mean.evaluate(Mean.java:131)
  at
org.apache.commons.math.stat.descriptive.DescriptiveStatisticsImpl.apply(DescriptiveStatisticsImpl.java:143)
  at
org.apache.commons.math.stat.descriptive.DescriptiveStatistics.getMean(DescriptiveStatistics.java:95)
  at
no.mypackage.spring.utils.ExecState.checkBadState(ExecState.java:89)
  at
no.mypackage.spring.utils.ExecutionTimeAdvice$3.execute(ExecutionTimeAdvice.java:179)
  at
org.apache.commons.collections.CollectionUtils.forAllDo(CollectionUtils.java:388)
  at
no.mypackage.spring.utils.ExecutionTimeAdvice.invoke(ExecutionTimeAdvice.java:174)
  at
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:171)
  at
org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:204)
  at $Proxy0.doSomething(Unknown Source)
  at
no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run(ExecutionTimeAdviceTest.java:47)
  at
EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run(Unknown Source)
  at java.lang.Thread.run(Thread.java:479)



If I change my code so I only call getMean() when getN >= windowSize it
seems to work OK.


part II of the code add's values (through the same object which has
synchronized access) - and this fails from time to time too (!):


java.lang.ArrayIndexOutOfBoundsException at
org.apache.commons.math.util.ResizableDoubleArray.addElement(ResizableDoubleArray.java:255)
  at
org.apache.commons.math.stat.descriptive.DescriptiveStatisticsImpl.addValue(DescriptiveStatisticsImpl.java:103)
  at no.dnbnor.spring.utils.ExecState.addExecTime(ExecState.java:68)
  at
no.mypackage.spring.utils.ExecutionTimeAdvice$4.execute(ExecutionTimeAdvice.java:203)
  at
org.apache.commons.collections.CollectionUtils.forAllDo(CollectionUtils.java:388)
  at
no.mypackage.spring.utils.ExecutionTimeAdvice.invoke(ExecutionTimeAdvice.java:200)
  at
org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:171)
  at
org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:204)
  at $Proxy0.doSomething(Unknown Source)
  at
no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run(ExecutionTimeAdviceTest.java:47)
  at
EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run(Unknown Source)
  at java.lang.Thread.run(Thread.java:479)



Any pointers???



David J. M. Karlsen - +47 90 68 22 43
http://www.davidkarlsen.com
http://mp3.davidkarlsen.com

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

Torsten Curdt
Sounds like a threading issue. I'd assume DescriptiveStatistics is  
not thread-safe and in your stacktrace I see

> at no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run
> (ExecutionTimeAdviceTest.java:47)
> at EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run
> (Unknown Source)
> at java.lang.Thread.run(Thread.java:479)

Try to synchronize access to the DescriptiveStatistics

cheers
--
Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

Brent Worden-2
David,

I just checked in a SynchronizedDescriptiveStatistics class that
should eliminate the threading issues you're experiencing.  Feel free
to grab it out of source control and give it a try.

http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/stat/descriptive/SynchronizedDescriptiveStatistics.java?view=markup


Other interested [math] parties,

To insure I didn't miss anything in my implementation, feel free to
look over this latest addition and critique it.

Thanks,

Brent.

On Oct 28, 2007 3:42 AM, Torsten Curdt <[hidden email]> wrote:

> Sounds like a threading issue. I'd assume DescriptiveStatistics is
> not thread-safe and in your stacktrace I see
>
> >       at no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run
> > (ExecutionTimeAdviceTest.java:47)
> >       at EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run
> > (Unknown Source)
> >       at java.lang.Thread.run(Thread.java:479)
>
> Try to synchronize access to the DescriptiveStatistics
>
> cheers
> --
> Torsten
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

sebb-2-2
On 31/10/2007, Brent Worden <[hidden email]> wrote:

> David,
>
> I just checked in a SynchronizedDescriptiveStatistics class that
> should eliminate the threading issues you're experiencing.  Feel free
> to grab it out of source control and give it a try.
>
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/stat/descriptive/SynchronizedDescriptiveStatistics.java?view=markup
>
>
> Other interested [math] parties,
>
> To insure I didn't miss anything in my implementation, feel free to
> look over this latest addition and critique it.

There are some spelling mistakes in the class description ;-)

It might be useful to add the annotation @ThreadSafe to the class,
e.g. as a comment:

/* @ThreadSafe */
public class ...

However, I'm not sure that the class is thread-safe.
For example the windowSize field is not final, and therefore may not
be visible to all threads after construction. Making it final would
fix this.

Fixing the eDA field might be tricky.

> Thanks,
>
> Brent.
>
> On Oct 28, 2007 3:42 AM, Torsten Curdt <[hidden email]> wrote:
> > Sounds like a threading issue. I'd assume DescriptiveStatistics is
> > not thread-safe and in your stacktrace I see
> >
> > >       at no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run
> > > (ExecutionTimeAdviceTest.java:47)
> > >       at EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run
> > > (Unknown Source)
> > >       at java.lang.Thread.run(Thread.java:479)
> >
> > Try to synchronize access to the DescriptiveStatistics
> >
> > cheers
> > --
> > Torsten
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

Rahul Akolkar
On 10/31/07, sebb <[hidden email]> wrote:

> On 31/10/2007, Brent Worden <[hidden email]> wrote:
> > David,
> >
> > I just checked in a SynchronizedDescriptiveStatistics class that
> > should eliminate the threading issues you're experiencing.  Feel free
> > to grab it out of source control and give it a try.
> >
> > http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/stat/descriptive/SynchronizedDescriptiveStatistics.java?view=markup
> >
> >
> > Other interested [math] parties,
> >
> > To insure I didn't miss anything in my implementation, feel free to
> > look over this latest addition and critique it.
>
> There are some spelling mistakes in the class description ;-)
>
> It might be useful to add the annotation @ThreadSafe to the class,
> e.g. as a comment:
>
> /* @ThreadSafe */
> public class ...
>
> However, I'm not sure that the class is thread-safe.
> For example the windowSize field is not final, and therefore may not
> be visible to all threads after construction. Making it final would
> fix this.
>
> Fixing the eDA field might be tricky.
>
<snip/>

Additionally, suggest couple of smallish changes:

 * r590566 [ http://svn.apache.org/viewvc?rev=590566&view=rev ]

Missing svn props (most importantly, svn:eol-style)

 * r590564 [ http://svn.apache.org/viewvc?rev=590564&view=rev ]

Modified: commons/proper/math/trunk/xdocs/userguide/stat.xml
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/xdocs/userguide/stat.xml?rev=590564&r1=590563&r2=590564&view=diff
==============================================================================
--- commons/proper/math/trunk/xdocs/userguide/stat.xml (original)
+++ commons/proper/math/trunk/xdocs/userguide/stat.xml Tue Oct 30 22:38:58 2007
@@ -199,7 +199,16 @@
 }
 in.close();
        </source>
-        </dd>
+        </dd>
+        <dt>Compute statistics in a thread-safe manner</dt>
+        <br/>
+        <dd>Use a <code>ThreadSafeDescriptiveStatistics</code> instance
+        <source>
<snip/>

Line above should say SynchronizedDescriptiveStatistics.


+// Create a SynchronizedDescriptiveStatistics instance and
+// use as any other DescriptiveStatistics instance
+DescriptiveStatistics stats =
DescriptiveStatistics.newInstance(SynchronizedDescriptiveStatistics.class);
+        </source>
+        </dd>
        </dl>
       </p>
      </subsection>




> > Thanks,
> >
> > Brent.
> >
> > On Oct 28, 2007 3:42 AM, Torsten Curdt <[hidden email]> wrote:
> > > Sounds like a threading issue. I'd assume DescriptiveStatistics is
> > > not thread-safe and in your stacktrace I see
> > >
> > > >       at no.mypackage.spring.utils.ExecutionTimeAdviceTest$1.run
> > > > (ExecutionTimeAdviceTest.java:47)
> > > >       at EDU.oswego.cs.dl.util.concurrent.PooledExecutor$Worker.run
> > > > (Unknown Source)
> > > >       at java.lang.Thread.run(Thread.java:479)
> > >
> > > Try to synchronize access to the DescriptiveStatistics
> > >
> > > cheers
> > > --
> > > Torsten
> > >

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

Brent Worden-2
In reply to this post by sebb-2-2
On Oct 31, 2007 6:41 AM, sebb <[hidden email]> wrote:
>
> However, I'm not sure that the class is thread-safe.
> For example the windowSize field is not final, and therefore may not
> be visible to all threads after construction. Making it final would
> fix this.
>
> Fixing the eDA field might be tricky.
>

I don't think this is a problem because no object reference to the
SynchronizedDescriptiveStatistics instance is published to other
objects during construction.  The first time an object reference is
made available to other objects (and other threads) is after the
constructor returns a new object.  At which time, the object is fully
constructed and all instance variables have been initialized.

Thanks,

Brent.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Trouble with commons-math

sebb-2-2
On 01/11/2007, Brent Worden <[hidden email]> wrote:

> On Oct 31, 2007 6:41 AM, sebb <[hidden email]> wrote:
> >
> > However, I'm not sure that the class is thread-safe.
> > For example the windowSize field is not final, and therefore may not
> > be visible to all threads after construction. Making it final would
> > fix this.
> >
> > Fixing the eDA field might be tricky.
> >
>
> I don't think this is a problem because no object reference to the
> SynchronizedDescriptiveStatistics instance is published to other
> objects during construction.  The first time an object reference is
> made available to other objects (and other threads) is after the
> constructor returns a new object.  At which time, the object is fully
> constructed and all instance variables have been initialized.
>

That's what I used to think, but then I read up on JSR133 and it seems
that non-final fields are not necessarily flushed to memory.

See for example Problem #1 in

http://www.ibm.com/developerworks/java/library/j-jtp02244.html?S_TACT=105AGX02&S_CMP=EDU

As I understand it, only final fields are guaranteed to be made
visible to other threads in the absence of any other synchronisation.

Note that Java 1.5 changes the String implementation to use final
fields. If that were not necessary, why was it done?

> Thanks,
>
> Brent.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]