On 7 September 2011 12:07, Gilles Sadowski <[email protected]> wrote: > On Wed, Sep 07, 2011 at 11:22:24AM +0100, sebb wrote: >> On 7 September 2011 10:46, Gilles Sadowski <[email protected]> >> wrote: >> > On Wed, Sep 07, 2011 at 12:42:06AM +0100, sebb wrote: >> >> On 7 September 2011 00:04, Gilles Sadowski <[email protected]> >> >> wrote: >> >> > Hello. >> >> > >> >> >> Modified: >> >> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math/util/FastMath.java >> >> >> URL: >> >> >> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/util/FastMath.java?rev=1165846&r1=1165845&r2=1165846&view=diff >> >> >> ============================================================================== >> >> >> --- >> >> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math/util/FastMath.java >> >> >> (original) >> >> >> +++ >> >> >> commons/proper/math/trunk/src/main/java/org/apache/commons/math/util/FastMath.java >> >> >> Tue Sep 6 21:06:58 2011 >> >> >> @@ -74,31 +74,6147 @@ public class FastMath { >> >> >> /** Napier's constant e, base of the natural logarithm. */ >> >> >> public static final double E = 2850325.0 / 1048576.0 + >> >> >> 8.254840070411028747e-8; >> >> >> >> >> >> + private static final int EXP_INT_TABLE_MAX_INDEX = 750; >> >> >> + private static final int EXP_INT_TABLE_LEN = >> >> >> EXP_INT_TABLE_MAX_INDEX * 2; >> >> >> + >> >> >> /** Exponential evaluated at integer values, >> >> >> - * exp(x) = expIntTableA[x + 750] + expIntTableB[x+750]. >> >> >> + * exp(x) = expIntTableA[x + EXP_INT_TABLE_MAX_INDEX] + >> >> >> expIntTableB[x+EXP_INT_TABLE_MAX_INDEX]. >> >> >> */ >> >> >> - private static final double EXP_INT_TABLE_A[] = new double[1500]; >> >> >> + private static final double EXP_INT_TABLE_A[] = >> >> >> + { >> >> >> + +0.0d, >> >> >> + Double.NaN, >> >> > >> >> > [More than 6000 lines stripped.] >> >> > >> >> > Wouldn't it be advantageous to store those tabulated data in separate >> >> > Java files? E.g. >> >> > --- >> >> > class ExpIntTables { >> >> > static final double[] A = { >> >> > // Very long table. >> >> > }; >> >> > static final double[] B = { >> >> > // ... >> >> > }; >> >> > --- >> >> > >> >> > And in "FastMath.java": >> >> > --- >> >> > public class FastMath { >> >> > private static final double[] EXP_INT_TABLE_A = ExpIntTables.A; >> >> > private static final double[] EXP_INT_TABLE_B = ExpIntTables.B; >> >> > } >> >> > --- >> >> >> >> That would be possible, but would require the tables to be >> >> non-private, increasing the theoretical risk of accidental changes. >> > >> > I know, but having "package" access level reduces the risk to our own >> > mistakes, similar to any bug we can introduce when writing code. >> >> Except that there are now many more files that may need to be debugged. > > I'm not sure I understand. Once the tables are generated, you put them in > their file and those data files need not be touched anymore.
I was referring to potential corruption of tables by other classes in the same package. >> >> > One can also assumes that the tables will need much less changes (if >> > at all) than actual code in "FastMath.java"; so having them in separate >> > files could reduce the risk of messing with them... And of course, there is >> > the practical advantage of not having to load/scroll 6000+ lines in orde to >> > have a look at the code. >> >> We could move the tables and disabled setup code to the end of the source >> file. > > I don't remember the exact setup, but I once run into a source code file > size problem by putting a lot of tables (as arrays) within the same Java > file. It was solved by separating them out in several files... > So, here one could imagine adding more and more tables (or adding more > entries to the existing ones) until this will occur. If it occurs we can fix the problem. The current change has added a few constants and some methods, but the number of tables and their contents are deliberately the same. >> >> Some of the setup code could be moved into a separate class - e.g. the >> slowExp, slowSin and SlowCos methods use the FACT table, which is not >> needed elsewhere. >> >> > By the way, how much faster is now the first use of a method from >> > "FastMath"? >> >> Not checked. Perhaps the OP of the JIRA will be able to help there. >> >> That probably needs to be the next step. > > If the benefit is not significant, I'd rather have the previous setup. That's currently fairly easy to revert. But it was necessary to make the change in order to test it. > > Gilles > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
