On 22 January 2015 at 01:59, Phil Steitz <phil.ste...@gmail.com> wrote: > On 1/21/15 4:56 PM, sebb wrote: >> On 21 January 2015 at 20:39, Thomas Neidhart <thomas.neidh...@gmail.com> >> wrote: >>> On 01/21/2015 05:32 PM, Phil Steitz wrote: >>>> On 1/21/15 3:43 AM, Thomas Neidhart wrote: >>>>> Hi, >>>>> >>>>> I have re-run the jenkins build for commons-math after this change several >>>>> times, also on H10 and it seems the test failures have disappeared. >>>>> >>>>> Any objection to keep this? >>>> -0 >>>> >>>> Is there a way to selectively suppress tests depending on JDK? >>> there are many different tests that failed. >>> We could either add a profile for java 1.5 to suppress them, or >>> something similar as in collections, were individual tests are skipped >>> when executed with a certain JVM due to known bugs. >> What effect does the change have on systems that don't have the JIT bug? > > Have a look at the diff below. Probably trivial, but non-zero per > impact, as it adds one compare for some extreme arguments.
Actually, what I should have written was: can the change affect the return value? Note: if it is possible to reliably (and cheaply) detect the likely presence of the bug, it might be possible to create a static final boolean which controls whether the extra test is needed or not. The JIT should optimise away any code that is conditional upon a boolean that is always false. Though looking at the code again, I wonder if intval needs to be forced positive. If not, this would avoid the need to check against Integer.MIN_VALUE. I did a quick check locally, and it seems to work OK for me on MacOSx. Obviously one has to change (intVal > 746) to (intVal < -746) etc. but intVal is not used much so the changes are minor When this is done, the setup of intPartA and intPartB is the same for both and can be done after the check for (x < 0.0). Though I would suggest the check should be changed to (intVal < 0) instead, as that is what now matters to the code in the conditional. Does that make sense or have I overlooked something? >> >> I tend to agree that fixing the code to avoid a bug in a single JVM is >> not ideal. >> >> Especially given that the bug only seems to happen on Java 5, and CM >> is likely to move away from that for the next release anyway. > > We will probably cut some more 3.x releases, so will want to be able > to test with 1.5. So probably Thoma' suggestion above is best. OK > Phil >> >>> Thomas >>> >>>> Phil >>>>> Thomas >>>>> >>>>> On Wed, Jan 21, 2015 at 12:42 AM, <t...@apache.org> wrote: >>>>> >>>>>> Repository: commons-math >>>>>> Updated Branches: >>>>>> refs/heads/master 4e1958256 -> 15bdcc3be >>>>>> >>>>>> >>>>>> Add temporary check for rare test failure. >>>>>> >>>>>> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo >>>>>> Commit: >>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/15bdcc3b >>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-math/tree/15bdcc3b >>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-math/diff/15bdcc3b >>>>>> >>>>>> Branch: refs/heads/master >>>>>> Commit: 15bdcc3be2b84f68ddcd822da52ef045ed89e57b >>>>>> Parents: 4e19582 >>>>>> Author: Thomas Neidhart <thomas.neidh...@gmail.com> >>>>>> Authored: Wed Jan 21 00:42:16 2015 +0100 >>>>>> Committer: Thomas Neidhart <thomas.neidh...@gmail.com> >>>>>> Committed: Wed Jan 21 00:42:16 2015 +0100 >>>>>> >>>>>> ---------------------------------------------------------------------- >>>>>> src/main/java/org/apache/commons/math3/util/FastMath.java | 6 +++++- >>>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>>> ---------------------------------------------------------------------- >>>>>> >>>>>> >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/15bdcc3b/src/main/java/org/apache/commons/math3/util/FastMath.java >>>>>> ---------------------------------------------------------------------- >>>>>> diff --git a/src/main/java/org/apache/commons/math3/util/FastMath.java >>>>>> b/src/main/java/org/apache/commons/math3/util/FastMath.java >>>>>> index 5178215..09c5976 100644 >>>>>> --- a/src/main/java/org/apache/commons/math3/util/FastMath.java >>>>>> +++ b/src/main/java/org/apache/commons/math3/util/FastMath.java >>>>>> @@ -876,7 +876,11 @@ public class FastMath { >>>>>> if (x < 0.0) { >>>>>> intVal = (int) -x; >>>>>> >>>>>> - if (intVal > 746) { >>>>>> + // TEMP: special handling of negative_infinity >>>>>> + // the above might fail in non-reproducible ways with Sun >>>>>> JDK >>>>>> 1.5, >>>>>> + // most likely due to a bug in the JIT. Add a safe-guard for >>>>>> very >>>>>> + // negative numbers. >>>>>> + if (intVal > 746 || x < Integer.MIN_VALUE) { >>>>>> if (hiPrec != null) { >>>>>> hiPrec[0] = 0.0; >>>>>> hiPrec[1] = 0.0; >>>>>> >>>>>> >>>> >>>> --------------------------------------------------------------------- >>>> 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 >>> >> --------------------------------------------------------------------- >> 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
