PERFORMANCE: Cleanup Redundant Timestamp instantiations

Fixes #7916

logstash-core/src/main/java/org/logstash/Event.java

@@ -314,11 +314,11 @@ public final class Event implements Cloneable, Queueable {
             } else if (o instanceof TimeBiValue) {
                 return new Timestamp(((TimeBiValue) o).javaValue());
             } else if (o instanceof JrubyTimestampExtLibrary.RubyTimestamp) {
-                return new Timestamp(((JrubyTimestampExtLibrary.RubyTimestamp) o).getTimestamp());
+                return ((JrubyTimestampExtLibrary.RubyTimestamp) o).getTimestamp();
             } else if (o instanceof Timestamp) {
-                return new Timestamp((Timestamp) o);
+                return (Timestamp) o;
             } else if (o instanceof TimestampBiValue) {
-                return new Timestamp(((TimestampBiValue) o).javaValue());
+                return ((TimestampBiValue) o).javaValue();
             } else if (o instanceof DateTime) {
                 return new Timestamp((DateTime) o);
             } else if (o instanceof Date) {

logstash-core/src/main/java/org/logstash/Timestamp.java

@@ -11,11 +11,16 @@ import org.joda.time.format.ISODateTimeFormat;
 import org.logstash.ackedqueue.Queueable;
 import org.logstash.json.TimestampSerializer;
 
+/**
+ * Wrapper around a {@link DateTime} with Logstash specific serialization behaviour.
+ * This class is immutable and thread-safe since its only state is held in a final {@link DateTime}
+ * reference and {@link DateTime} which itself is immutable and thread-safe.
+ */
 @JsonSerialize(using = TimestampSerializer.class)
-public final class Timestamp implements Cloneable, Comparable<Timestamp>, Queueable {
+public final class Timestamp implements Comparable<Timestamp>, Queueable {
 
     // all methods setting the time object must set it in the UTC timezone
-    private DateTime time;
+    private final DateTime time;
 
     private static final DateTimeFormatter iso8601Formatter = ISODateTimeFormat.dateTime();
 
@@ -29,18 +34,10 @@ public final class Timestamp implements Cloneable, Comparable<Timestamp>, Queuea
         this.time = ISODateTimeFormat.dateTimeParser().parseDateTime(iso8601).toDateTime(DateTimeZone.UTC);
     }
 
-    public Timestamp(Timestamp t) {
-        this.time = t.getTime();
-    }
-
     public Timestamp(long epoch_milliseconds) {
         this.time = new DateTime(epoch_milliseconds, DateTimeZone.UTC);
     }
 
-    public Timestamp(Long epoch_milliseconds) {
-        this.time = new DateTime(epoch_milliseconds, DateTimeZone.UTC);
-    }
-
     public Timestamp(Date date) {
         this.time = new DateTime(date, DateTimeZone.UTC);
     }
@@ -53,10 +50,6 @@ public final class Timestamp implements Cloneable, Comparable<Timestamp>, Queuea
         return time;
     }
 
-    public void setTime(DateTime time) {
-        this.time = time.toDateTime(DateTimeZone.UTC);
-    }
-
     public static Timestamp now() {
         return new Timestamp();
     }
@@ -77,14 +70,7 @@ public final class Timestamp implements Cloneable, Comparable<Timestamp>, Queuea
 
     @Override
     public int compareTo(Timestamp other) {
-        return getTime().compareTo(other.getTime());
+        return time.compareTo(other.time);
-    }
-
-    @Override
-    public Timestamp clone() throws CloneNotSupportedException {
-        Timestamp clone = (Timestamp)super.clone();
-        clone.setTime(this.getTime());
-        return clone;
     }
 
     @Override

logstash-core/src/main/java/org/logstash/ext/JrubyTimestampExtLibrary.java

@@ -159,28 +159,22 @@ public class JrubyTimestampExtLibrary implements Library {
             return RubyString.newString(context.runtime,  "\"" + this.timestamp.toIso8601() + "\"");
         }
 
-        public static Timestamp newTimestamp(IRubyObject time)
-        {
-            if (time.isNil()) {
-                return new Timestamp();
-            } else if (time instanceof RubyTime) {
-                return new Timestamp(((RubyTime)time).getDateTime());
-            } else if (time instanceof RubyString) {
-                return new Timestamp(time.toString());
-            } else if (time instanceof RubyTimestamp) {
-                return new Timestamp(((RubyTimestamp) time).timestamp);
-            } else {
-               return null;
-            }
-        }
-
-
         @JRubyMethod(name = "coerce", required = 1, meta = true)
         public static IRubyObject ruby_coerce(ThreadContext context, IRubyObject recv, IRubyObject time)
         {
             try {
-                Timestamp ts = newTimestamp(time);
+                if (time instanceof RubyTimestamp) {
-                return (ts == null) ? context.runtime.getNil() : RubyTimestamp.newRubyTimestamp(context.runtime, ts);
+                    return time;
+                } else if (time instanceof RubyTime) {
+                    return RubyTimestamp.newRubyTimestamp(
+                        context.runtime,
+                        new Timestamp(((RubyTime) time).getDateTime())
+                    );
+                } else if (time instanceof RubyString) {
+                    return fromRString(context.runtime, (RubyString) time);
+                } else {
+                    return context.runtime.getNil();
+                }
              } catch (IllegalArgumentException e) {
                 throw new RaiseException(
                         context.runtime,
@@ -197,7 +191,7 @@ public class JrubyTimestampExtLibrary implements Library {
         {
             if (time instanceof RubyString) {
                 try {
-                    return RubyTimestamp.newRubyTimestamp(context.runtime, newTimestamp(time));
+                    return fromRString(context.runtime, (RubyString) time);
                 } catch (IllegalArgumentException e) {
                     throw new RaiseException(
                             context.runtime,
@@ -254,5 +248,9 @@ public class JrubyTimestampExtLibrary implements Library {
         {
             return RubyFixnum.newFixnum(context.runtime, this.timestamp.getTime().getYear());
         }
+
+        private static RubyTimestamp fromRString(final Ruby runtime, final RubyString string) {
+            return RubyTimestamp.newRubyTimestamp(runtime, new Timestamp(string.toString()));
+        }
     }
 }

logstash-core/src/test/java/org/logstash/TimestampTest.java

@@ -36,12 +36,9 @@ public class TimestampTest {
         t = new Timestamp("2014-09-23T08:00:00.000Z");
         assertEquals(DateTimeZone.UTC, t.getTime().getZone());
 
-        t = new Timestamp(new Timestamp());
-        assertEquals(DateTimeZone.UTC, t.getTime().getZone());
-
         long ms = DateTime.now(DateTimeZone.forID("EST")).getMillis();
         t = new Timestamp(ms);
         assertEquals(DateTimeZone.UTC, t.getTime().getZone());
     }
 
-}
+}

logstash-core/src/test/java/org/logstash/common/io/DeadLetterQueueReaderTest.java

@@ -307,7 +307,7 @@ public class DeadLetterQueueReaderTest {
 
     private void seekReadAndVerify(final Timestamp seekTarget, final String expectedValue) throws Exception {
         try(DeadLetterQueueReader readManager = new DeadLetterQueueReader(dir)) {
-            readManager.seekToNextEvent(new Timestamp(seekTarget));
+            readManager.seekToNextEvent(seekTarget);
             DLQEntry readEntry = readManager.pollEntry(100);
             assertThat(readEntry.getReason(), equalTo(expectedValue));
             assertThat(readEntry.getEntryTime().toIso8601(), equalTo(seekTarget.toIso8601()));