commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jake Mannix <jake.man...@gmail.com>
Subject Re: [math] Performance optimization in the vector classes
Date Mon, 02 Nov 2009 09:49:48 GMT
Hey all,

  In digging through ArrayRealVector and OpenMapRealVector, while trying to
draw up patches for MATH-312 and MATH-314, I found a number of performance (
and other) issues:

1) in add(double[]), subtract(double[]), and mapXXX methods, the idiom is:

  double[] v = new double[data.length];
  for(int i=0; i<v.length; i++) {
    v[i] = data[i] + otherVector[i]; // for example, for add()
  }
  return new ArrayRealVector(v);

if this were instead:

  double[] v = data.clone();
  for(int i=0; i<v.length; i++) {
    v[i] += otherVector[i];
  }
  return new ArrayRealVector(v, false);   // <- ack!  not passing false here
forces a clone() call when you least need it!

The difference of doing only one array access inside the loop alone saved
30% on profiling tests on my laptop with 1M long double arrays.  I don't
even want to think about the cost of making superflous array copies as well.

2) In OpenMapRealVector, any case where two OpenMapRealVectors are combined
together, iterating over the smaller should be preferred, but this is never
checked for.

3) what's with the idiom of:

  try { return methodName( (ArrayRealVector) v); } catch (ClassCastException
cce) {

instead of the standard

  if(v instanceof ArrayRealVector) { return methodName( (ArrayRealVector)
v); } else {

Is there something I'm missing on why the former shows up all over the
place?  Maybe this isn't a performance issue, but I just noticed it.

4) ArrayRealVector#unitVector() calls norm() once, and then it calls it
again.  That's clearly unnecessary.  Also, why the special case for
ArithmeticExcption here, for the zero norm case?  Why not just let java just
try to divide, and if it divides by zero, well, it'll throw an
ArithmeticException itself...  seems like the whole method should just be
implemented as " return mapDivide(norm()); "

I've fixed many of these in my patch on MATH-312, but there's another
version of that I need to post once I correctly handle the sparse nonzero
defaultValue case.   I also wasn't sure if you guys usually liked to
separate your fixes into a bunch of little ones or not.  It's just hard for
me to keep a dozen different patches in sync my end without conflicting with
myself now and again.

  -jake

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message