tmf.core: fix timestamp normalization clamping
authorMatthew Khouzam <matthew.khouzam@ericsson.com>
Sun, 28 Feb 2016 14:57:34 +0000 (09:57 -0500)
committerMatthew Khouzam <matthew.khouzam@ericsson.com>
Fri, 4 Mar 2016 01:10:37 +0000 (20:10 -0500)
This patch fixes a behavior issue with timestamps. Offsetting and
normalizing timestamps no longer overflows, nor does it throw
arithmetic exceptions.

This is done by using a saturated add and mult method from TmfTimestamp.
The saturated math should have a negligeable impact on performance.

Change-Id: I8f6fc7fc930586897923bb8510574ec4c4c668b3
Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/67501
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-by: Hudson CI
tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/event/TmfTimestampTest.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/timestamp/TmfNanoTimestamp.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/timestamp/TmfSimpleTimestamp.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/timestamp/TmfTimestamp.java

index d2401c7c01e02d26f28de273a375a842e6555634..3212b35fd89e6f98aa22b23a94a196f6c17296c6 100644 (file)
@@ -18,7 +18,6 @@ package org.eclipse.tracecompass.tmf.core.tests.event;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
@@ -40,15 +39,17 @@ public class TmfTimestampTest {
     // ------------------------------------------------------------------------
 
     private final ITmfTimestamp ts0 = new TmfTimestamp();
-    private final ITmfTimestamp ts1 = new TmfTimestamp(12345,  0);
+    private final ITmfTimestamp ts1 = new TmfTimestamp(12345, 0);
     private final ITmfTimestamp ts2 = new TmfTimestamp(12345, -1);
-    private final ITmfTimestamp ts3 = new TmfTimestamp(12345,  2);
+    private final ITmfTimestamp ts3 = new TmfTimestamp(12345, 2);
     private final ITmfTimestamp ts4 = new TmfTimestamp(12345, -3);
     private final ITmfTimestamp ts5 = new TmfTimestamp(12345, -6);
     private final ITmfTimestamp ts6 = new TmfTimestamp(12345, -9);
     private final ITmfTimestamp ts7 = new TmfTimestamp(-12345, -3);
     private final ITmfTimestamp ts8 = new TmfTimestamp(-12345, -6);
     private final ITmfTimestamp ts9 = new TmfTimestamp(-12345, -9);
+    private final ITmfTimestamp ts10 = new TmfTimestamp(Long.MAX_VALUE / 100, -6);
+    private final ITmfTimestamp ts11 = new TmfTimestamp(Long.MIN_VALUE / 100, -6);
 
     // ------------------------------------------------------------------------
     // Constructors
@@ -90,7 +91,7 @@ public class TmfTimestampTest {
         assertEquals("getscale", 2, copy.getScale());
     }
 
-    @Test(expected=IllegalArgumentException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testCopyNullConstructor() {
         new TmfTimestamp((TmfTimestamp) null);
     }
@@ -309,30 +310,16 @@ public class TmfTimestampTest {
         final int MAX_SCALE_DIFF = 19;
 
         // Test below limit
-        try {
-            ts1.normalize(0, +MAX_SCALE_DIFF - 1);
-            ts1.normalize(0, -MAX_SCALE_DIFF + 1);
-        } catch (final ArithmeticException e) {
-            fail("normalize: scale error");
-        }
+        assertEquals(Long.MAX_VALUE, ts1.normalize(0, -MAX_SCALE_DIFF + 1).getValue());
+        assertEquals(0, ts1.normalize(0, +MAX_SCALE_DIFF - 1).getValue());
 
         // Test at limit
-        try {
-            ts1.normalize(0, +MAX_SCALE_DIFF);
-            fail("normalize: scale error");
-            ts1.normalize(0, -MAX_SCALE_DIFF);
-            fail("normalize: scale error");
-        } catch (final ArithmeticException e) {
-        }
+        assertEquals(Long.MAX_VALUE, ts1.normalize(0, -MAX_SCALE_DIFF).getValue());
+        assertEquals(0, ts1.normalize(0, +MAX_SCALE_DIFF).getValue());
 
         // Test over limit
-        try {
-            ts1.normalize(0, +MAX_SCALE_DIFF + 1);
-            fail("normalize: scale error");
-            ts1.normalize(0, -MAX_SCALE_DIFF - 1);
-            fail("normalize: scale error");
-        } catch (final ArithmeticException e) {
-        }
+        assertEquals(Long.MAX_VALUE, ts1.normalize(0, -MAX_SCALE_DIFF - 1).getValue());
+        assertEquals(0, ts1.normalize(0, +MAX_SCALE_DIFF - 1).getValue());
     }
 
     @Test
@@ -384,6 +371,14 @@ public class TmfTimestampTest {
         assertEquals("getscale", SCALE, ts.getScale());
     }
 
+    @Test
+    public void testToNanos() {
+        assertEquals(12345000000000L, ts1.toNanos());
+        assertEquals(-12345, ts9.toNanos());
+        assertEquals(Long.MAX_VALUE, ts10.toNanos());
+        assertEquals(Long.MIN_VALUE, ts11.toNanos());
+    }
+
     // ------------------------------------------------------------------------
     // compareTo
     // ------------------------------------------------------------------------
@@ -430,14 +425,14 @@ public class TmfTimestampTest {
         final ITmfTimestamp ts0b = new TmfTimestamp(0, Integer.MAX_VALUE);
         final ITmfTimestamp ts0c = new TmfTimestamp(Long.MAX_VALUE, Integer.MAX_VALUE);
 
-        assertTrue("compareTo", ts0a.compareTo(ts0b) == 1);
-        assertTrue("compareTo", ts0a.compareTo(ts0c) == -1);
+        assertEquals("compareTo", 1, ts0a.compareTo(ts0b));
+        assertEquals("compareTo", -1, ts0a.compareTo(ts0c));
 
-        assertTrue("compareTo", ts0b.compareTo(ts0a) == -1);
-        assertTrue("compareTo", ts0b.compareTo(ts0c) == -1);
+        assertEquals("compareTo", -1, ts0b.compareTo(ts0a));
+        assertEquals("compareTo", -1, ts0b.compareTo(ts0c));
 
-        assertTrue("compareTo", ts0c.compareTo(ts0a) == 1);
-        assertTrue("compareTo", ts0c.compareTo(ts0b) == 1);
+        assertEquals("compareTo", 1, ts0c.compareTo(ts0a));
+        assertEquals("compareTo", 1, ts0c.compareTo(ts0b));
     }
 
     @Test
@@ -446,19 +441,19 @@ public class TmfTimestampTest {
         final ITmfTimestamp ts0b = new TmfTimestamp(0, Integer.MAX_VALUE);
         final ITmfTimestamp ts0c = new TmfTimestamp(Long.MIN_VALUE, Integer.MAX_VALUE);
 
-        assertTrue("compareTo", ts0a.compareTo(ts0b) == -1);
-        assertTrue("compareTo", ts0a.compareTo(ts0c) == 1);
+        assertEquals("compareTo", -1, ts0a.compareTo(ts0b));
+        assertEquals("compareTo", 1, ts0a.compareTo(ts0c));
 
-        assertTrue("compareTo", ts0b.compareTo(ts0a) == 1);
-        assertTrue("compareTo", ts0b.compareTo(ts0c) == 1);
+        assertEquals("compareTo", 1, ts0b.compareTo(ts0a));
+        assertEquals("compareTo", 1, ts0b.compareTo(ts0c));
 
-        assertTrue("compareTo", ts0c.compareTo(ts0a) == -1);
-        assertTrue("compareTo", ts0c.compareTo(ts0b) == -1);
+        assertEquals("compareTo", -1, ts0c.compareTo(ts0a));
+        assertEquals("compareTo", -1, ts0c.compareTo(ts0b));
     }
 
     @Test
     public void testCompareToCornerCases4() {
-        assertTrue("compareTo", ts0.compareTo(null) == 1);
+        assertEquals("compareTo", 1, ts0.compareTo(null));
     }
 
     @Test
@@ -468,7 +463,7 @@ public class TmfTimestampTest {
         final ITmfTimestamp t3 = new TmfTimestamp(1100, 0);
         final ITmfTimestamp t4 = new TmfTimestamp(1000, 0);
 
-        assertTrue(t1.compareTo(t1) == 0);
+        assertEquals(0, t1.compareTo(t1));
 
         assertTrue("CompareTo", t1.compareTo(t2) < 0);
         assertTrue("CompareTo", t1.compareTo(t3) < 0);
@@ -512,7 +507,7 @@ public class TmfTimestampTest {
         final ITmfTimestamp t3 = new TmfTimestamp(1, 100);
         final ITmfTimestamp t4 = new TmfTimestamp(1000, -100);
 
-        assertTrue("CompareTo", t1.compareTo(t2) < 0);
+        assertEquals("CompareTo", -1, t1.compareTo(t2));
         assertTrue("CompareTo", t1.compareTo(t3) < 0);
         assertTrue("CompareTo", t1.compareTo(t4) < 0);
 
@@ -534,11 +529,11 @@ public class TmfTimestampTest {
         final ITmfTimestamp ts0a = new TmfTimestamp(0, Integer.MAX_VALUE);
         final ITmfTimestamp ts0b = new TmfTimestamp(1, Integer.MAX_VALUE);
 
-        assertTrue("CompareTo", ts0a.compareTo(ts0) == 0);
-        assertTrue("CompareTo", ts0.compareTo(ts0a) == 0);
+        assertEquals("CompareTo", 0, ts0a.compareTo(ts0));
+        assertEquals("CompareTo", 0, ts0.compareTo(ts0a));
 
-        assertTrue("CompareTo", ts0b.compareTo(ts0) == 1);
-        assertTrue("CompareTo", ts0.compareTo(ts0b) == -1);
+        assertEquals("CompareTo", 1, ts0b.compareTo(ts0));
+        assertEquals("CompareTo", -1, ts0.compareTo(ts0b));
     }
 
     // ------------------------------------------------------------------------
index 9ae49c1dd7a3eaa06c0ea424bed9c105b02d23a9..150ba09e83fc98e553c0fdda282dbca7eb9e63c8 100644 (file)
@@ -59,7 +59,7 @@ public class TmfNanoTimestamp extends TmfTimestamp {
     @Override
     public ITmfTimestamp normalize(final long offset, final int scale) {
         if (scale == ITmfTimestamp.NANOSECOND_SCALE) {
-            return new TmfNanoTimestamp(getValue() + offset);
+            return new TmfNanoTimestamp(saturatedAdd(getValue(), offset));
         }
         return super.normalize(offset, scale);
     }
@@ -85,7 +85,7 @@ public class TmfNanoTimestamp extends TmfTimestamp {
      * @since 2.0
      */
     @Override
-    public long toNanos(){
+    public long toNanos() {
         return getValue();
     }
 
index e0a20173eafd78710fe40cb0251ae0fb147d3c15..78cce97c6b443f18861c90571638851eb15cc617 100644 (file)
@@ -59,8 +59,8 @@ public class TmfSimpleTimestamp extends TmfTimestamp {
 
     @Override
     public ITmfTimestamp normalize(final long offset, final int scale) {
-        if (scale == 0) {
-            return new TmfSimpleTimestamp(getValue() + offset);
+        if (scale == ITmfTimestamp.SECOND_SCALE) {
+            return new TmfSimpleTimestamp(saturatedAdd(getValue(), offset));
         }
         return super.normalize(offset, scale);
     }
index 422a2736b71182ea57da57ab396af0b5cdab1256..e1102ded77683aeb02fc3106785d885d9a36313a 100644 (file)
@@ -20,9 +20,8 @@ import java.nio.ByteBuffer;
 import org.eclipse.jdt.annotation.NonNull;
 
 /**
- * A generic timestamp implementation. The timestamp is represented by the
- * tuple { value, scale, precision }. By default, timestamps are scaled in
- * seconds.
+ * A generic timestamp implementation. The timestamp is represented by the tuple
+ * { value, scale, precision }. By default, timestamps are scaled in seconds.
  *
  * @author Francois Chouinard
  */
@@ -35,20 +34,17 @@ public class TmfTimestamp implements ITmfTimestamp {
     /**
      * The beginning of time
      */
-    public static final @NonNull ITmfTimestamp BIG_BANG =
-            new TmfTimestamp(Long.MIN_VALUE, Integer.MAX_VALUE);
+    public static final @NonNull ITmfTimestamp BIG_BANG = new TmfTimestamp(Long.MIN_VALUE, Integer.MAX_VALUE);
 
     /**
      * The end of time
      */
-    public static final @NonNull ITmfTimestamp BIG_CRUNCH =
-            new TmfTimestamp(Long.MAX_VALUE, Integer.MAX_VALUE);
+    public static final @NonNull ITmfTimestamp BIG_CRUNCH = new TmfTimestamp(Long.MAX_VALUE, Integer.MAX_VALUE);
 
     /**
      * Zero
      */
-    public static final @NonNull ITmfTimestamp ZERO =
-            new TmfTimestamp(0, 0);
+    public static final @NonNull ITmfTimestamp ZERO = new TmfTimestamp(0, 0);
 
     // ------------------------------------------------------------------------
     // Attributes
@@ -153,63 +149,67 @@ public class TmfTimestamp implements ITmfTimestamp {
     }
 
     private static final long scalingFactors[] = new long[] {
-        1L,
-        10L,
-        100L,
-        1000L,
-        10000L,
-        100000L,
-        1000000L,
-        10000000L,
-        100000000L,
-        1000000000L,
-        10000000000L,
-        100000000000L,
-        1000000000000L,
-        10000000000000L,
-        100000000000000L,
-        1000000000000000L,
-        10000000000000000L,
-        100000000000000000L,
-        1000000000000000000L,
+            1L,
+            10L,
+            100L,
+            1000L,
+            10000L,
+            100000L,
+            1000000L,
+            10000000L,
+            100000000L,
+            1000000000L,
+            10000000000L,
+            100000000000L,
+            1000000000000L,
+            10000000000000L,
+            100000000000000L,
+            1000000000000000L,
+            10000000000000000L,
+            100000000000000000L,
+            1000000000000000000L,
     };
 
     @Override
     public ITmfTimestamp normalize(final long offset, final int scale) {
 
-        long value = fValue;
+        long value = getValue();
 
         // Handle the trivial case
-        if (fScale == scale && offset == 0) {
+        if (getScale() == scale && offset == 0) {
             return this;
         }
 
-        // In case of big bang and big crunch just return this (no need to normalize)
+        // In case of big bang and big crunch just return this (no need to
+        // normalize)
         if (this.equals(BIG_BANG) || this.equals(BIG_CRUNCH)) {
             return this;
         }
 
+        if (value == 0) {
+            return new TmfTimestamp(offset, scale);
+        }
+
         // First, scale the timestamp
-        if (fScale != scale) {
-            final int scaleDiff = Math.abs(fScale - scale);
+        if (getScale() != scale) {
+            final int scaleDiff = Math.abs(getScale() - scale);
             if (scaleDiff >= scalingFactors.length) {
-                throw new ArithmeticException("Scaling exception"); //$NON-NLS-1$
-            }
-
-            final long scalingFactor = scalingFactors[scaleDiff];
-            if (scale < fScale) {
-                value *= scalingFactor;
+                if (getScale() < scale) {
+                    value = 0;
+                } else {
+                    value = value > 0 ? Long.MAX_VALUE : Long.MIN_VALUE;
+                }
             } else {
-                value /= scalingFactor;
+                final long scalingFactor = scalingFactors[scaleDiff];
+                if (getScale() < scale) {
+                    value /= scalingFactor;
+                } else {
+                    value = saturatedMult(scalingFactor, value);
+                }
             }
         }
 
-        // Then, apply the offset
-        if (offset < 0) {
-            value = (value < Long.MIN_VALUE - offset) ? Long.MIN_VALUE : value + offset;
-        } else {
-            value = (value > Long.MAX_VALUE - offset) ? Long.MAX_VALUE : value + offset;
-        }
+        value = saturatedAdd(value, offset);
 
         return new TmfTimestamp(value, scale);
     }
@@ -236,7 +236,8 @@ public class TmfTimestamp implements ITmfTimestamp {
 
     @Override
     public int compareTo(final ITmfTimestamp ts) {
-        // Check the corner cases (we can't use equals() because it uses compareTo()...)
+        // Check the corner cases (we can't use equals() because it uses
+        // compareTo()...)
         if (ts == null) {
             return 1;
         }
@@ -249,13 +250,8 @@ public class TmfTimestamp implements ITmfTimestamp {
         if ((fValue == BIG_CRUNCH.getValue() && fScale == BIG_CRUNCH.getScale()) || (ts.getValue() == BIG_BANG.getValue() && ts.getScale() == BIG_BANG.getScale())) {
             return 1;
         }
-
-        try {
-            final ITmfTimestamp nts = ts.normalize(0, fScale);
-            final long delta = fValue - nts.getValue();
-            return Long.compare(delta, 0);
-        }
-        catch (final ArithmeticException e) {
+        final ITmfTimestamp nts = ts.normalize(0, fScale);
+        if ((nts.getValue() == 0 && ts.getValue() != 0) || (ts.getValue() != Long.MAX_VALUE && nts.getValue() == Long.MAX_VALUE) || (ts.getValue() != Long.MIN_VALUE && nts.getValue() == Long.MIN_VALUE)) {
             // Scaling error. We can figure it out nonetheless.
 
             // First, look at the sign of the mantissa
@@ -274,6 +270,58 @@ public class TmfTimestamp implements ITmfTimestamp {
             final int scale = ts.getScale();
             return (fScale > scale) ? (fValue >= 0) ? 1 : -1 : (fValue >= 0) ? -1 : 1;
         }
+        final long delta = fValue - nts.getValue();
+        return Long.compare(delta, 0);
+    }
+
+    /**
+     * Saturated multiplication. It will not overflow but instead clamp the
+     * result to {@link Long#MAX_VALUE} and {@link Long#MIN_VALUE}.
+     *
+     * @param left
+     *            The left long to multiply
+     * @param right
+     *            The right long to multiply
+     * @return The saturated multiplication result. The mathematical, not Java
+     *         version of Min(Max(MIN_VALUE, left*right), MAX_VALUE).
+     * @see <a href="http://en.wikipedia.org/wiki/Saturation_arithmetic">
+     *      Saturation arithmetic</a>
+     */
+    private static long saturatedMult(long left, long right) {
+        long retVal = left * right;
+        if ((left != 0) && ((retVal / left) != right)) {
+            return (sameSign(left, right) ? Long.MAX_VALUE : Long.MIN_VALUE);
+        }
+        return retVal;
+    }
+
+    /**
+     * Saturated addition. It will not overflow but instead clamp the result to
+     * {@link Long#MAX_VALUE} and {@link Long#MIN_VALUE}.
+     *
+     * @param left
+     *            The left long to add
+     * @param right
+     *            The right long to add
+     * @return The saturated addition result. The mathematical, not Java version
+     *         of Min(Max(MIN_VALUE, left+right), MAX_VALUE).
+     * @see <a href="http://en.wikipedia.org/wiki/Saturation_arithmetic">
+     *      Saturation arithmetic</a>
+     * @since 2.0
+     */
+    protected static final long saturatedAdd(final long left, final long right) {
+        long retVal = left + right;
+        if (sameSign(left, right) && !sameSign(left, retVal)) {
+            if (retVal > 0) {
+                return Long.MIN_VALUE;
+            }
+            return Long.MAX_VALUE;
+        }
+        return retVal;
+    }
+
+    private static boolean sameSign(final long left, final long right) {
+        return (left ^ right) >= 0;
     }
 
     // ------------------------------------------------------------------------
@@ -314,15 +362,16 @@ public class TmfTimestamp implements ITmfTimestamp {
     public String toString(final TmfTimestampFormat format) {
         try {
             return format.format(toNanos());
-        }
-        catch (ArithmeticException e) {
+        } catch (ArithmeticException e) {
             return format.format(0);
         }
     }
 
     /**
      * Write the time stamp to the ByteBuffer so that it can be saved to disk.
-     * @param bufferOut the buffer to write to
+     *
+     * @param bufferOut
+     *            the buffer to write to
      */
     public void serialize(ByteBuffer bufferOut) {
         bufferOut.putLong(fValue);
This page took 0.034046 seconds and 5 git commands to generate.