From f80a5dc4efc46e81ac11fd4e61856c062fdec5a0 Mon Sep 17 00:00:00 2001
From: Nishant <nishant.monu51@gmail.com>
Date: Tue, 19 Apr 2016 23:00:14 +0530
Subject: [PATCH] Avoid creating multiple NoneShardSpec objects (#2855)

* Avoid creating multiple NoneShardSpec objects
* deprecate NoneShardSpec constructor
---
 .../java/io/druid/timeline/DataSegment.java     |  4 ++--
 .../druid/timeline/partition/NoneShardSpec.java | 17 ++++++++++++-----
 .../timeline/partition/NoneShardSpecTest.java   | 16 ++++++++++++++++
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/api/src/main/java/io/druid/timeline/DataSegment.java b/api/src/main/java/io/druid/timeline/DataSegment.java
index 97e9fffda72..5b874d62204 100644
--- a/api/src/main/java/io/druid/timeline/DataSegment.java
+++ b/api/src/main/java/io/druid/timeline/DataSegment.java
@@ -126,7 +126,7 @@ public class DataSegment implements Comparable<DataSegment>
     this.metrics = metrics == null
                    ? ImmutableList.<String>of()
                    : ImmutableList.copyOf(Iterables.transform(Iterables.filter(metrics, nonEmpty), internFun));
-    this.shardSpec = (shardSpec == null) ? new NoneShardSpec() : shardSpec;
+    this.shardSpec = (shardSpec == null) ? NoneShardSpec.instance() : shardSpec;
     this.binaryVersion = binaryVersion;
     this.size = size;
 
@@ -328,7 +328,7 @@ public class DataSegment implements Comparable<DataSegment>
       this.loadSpec = ImmutableMap.of();
       this.dimensions = ImmutableList.of();
       this.metrics = ImmutableList.of();
-      this.shardSpec = new NoneShardSpec();
+      this.shardSpec = NoneShardSpec.instance();
       this.size = -1;
     }
 
diff --git a/api/src/main/java/io/druid/timeline/partition/NoneShardSpec.java b/api/src/main/java/io/druid/timeline/partition/NoneShardSpec.java
index e1b7eb42917..e33dd2afc85 100644
--- a/api/src/main/java/io/druid/timeline/partition/NoneShardSpec.java
+++ b/api/src/main/java/io/druid/timeline/partition/NoneShardSpec.java
@@ -19,19 +19,26 @@
 
 package io.druid.timeline.partition;
 
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterables;
-import com.metamx.common.ISE;
+import com.fasterxml.jackson.annotation.JsonCreator;
 import io.druid.data.input.InputRow;
 
-import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
 /**
  */
 public class NoneShardSpec implements ShardSpec
 {
+  private final static NoneShardSpec INSTANCE = new NoneShardSpec();
+
+  @JsonCreator
+  public static NoneShardSpec instance() { return INSTANCE; }
+
+  @Deprecated
+  // Use NoneShardSpec.instance() instead
+  public NoneShardSpec(){
+
+  }
+
   @Override
   public <T> PartitionChunk<T> createChunk(T obj)
   {
diff --git a/api/src/test/java/io/druid/timeline/partition/NoneShardSpecTest.java b/api/src/test/java/io/druid/timeline/partition/NoneShardSpecTest.java
index 887b1f593c1..0c3035471d2 100644
--- a/api/src/test/java/io/druid/timeline/partition/NoneShardSpecTest.java
+++ b/api/src/test/java/io/druid/timeline/partition/NoneShardSpecTest.java
@@ -1,5 +1,8 @@
 package io.druid.timeline.partition;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.druid.TestObjectMapper;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -13,4 +16,17 @@ public class NoneShardSpecTest
     Assert.assertEquals(one, two);
     Assert.assertEquals(one.hashCode(), two.hashCode());
   }
+
+  @Test
+  public void testSerde() throws Exception
+  {
+    final NoneShardSpec one = NoneShardSpec.instance();
+    ObjectMapper mapper = new TestObjectMapper();
+    NoneShardSpec serde1 = mapper.readValue(mapper.writeValueAsString(one), NoneShardSpec.class);
+    NoneShardSpec serde2 = mapper.readValue(mapper.writeValueAsString(one), NoneShardSpec.class);
+
+    // Serde should return same object instead of creating new one every time.
+    Assert.assertTrue(serde1 == serde2);
+    Assert.assertTrue(one == serde1);
+  }
 }