Reduce cost of various toFilter calls. (#2860)

These happen once per segment and so it's better if they don't do
as much work.

processing/src/main/java/io/druid/query/filter/InDimFilter.java

@@ -24,24 +24,19 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import com.metamx.common.StringUtils;
 import io.druid.query.extraction.ExtractionFn;
-import io.druid.query.lookup.LookupExtractionFn;
-import io.druid.query.lookup.LookupExtractor;
 import io.druid.segment.filter.InFilter;
 
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Objects;
+import java.util.Set;
 
 public class InDimFilter implements DimFilter
 {
-  private final List<String> values;
+  private final ImmutableSortedSet<String> values;
   private final String dimension;
   private final ExtractionFn extractionFn;
 
@@ -54,7 +49,7 @@ public class InDimFilter implements DimFilter
   {
     Preconditions.checkNotNull(dimension, "dimension can not be null");
     Preconditions.checkArgument(values != null && !values.isEmpty(), "values can not be null or empty");
-    this.values = Lists.newArrayList(
+    this.values = ImmutableSortedSet.copyOf(
         Iterables.transform(
             values, new Function<String, String>()
             {
@@ -78,7 +73,7 @@ public class InDimFilter implements DimFilter
   }
 
   @JsonProperty
-  public List<String> getValues()
+  public Set<String> getValues()
   {
     return values;
   }
@@ -103,7 +98,10 @@ public class InDimFilter implements DimFilter
     }
     byte[] extractionFnBytes = extractionFn == null ? new byte[0] : extractionFn.getCacheKey();
 
-    ByteBuffer filterCacheKey = ByteBuffer.allocate(3 + dimensionBytes.length + valuesBytesSize + extractionFnBytes.length)
+    ByteBuffer filterCacheKey = ByteBuffer.allocate(3
+                                                    + dimensionBytes.length
+                                                    + valuesBytesSize
+                                                    + extractionFnBytes.length)
                                           .put(DimFilterCacheHelper.IN_CACHE_ID)
                                           .put(dimensionBytes)
                                           .put(DimFilterCacheHelper.STRING_SEPARATOR)
@@ -125,7 +123,7 @@ public class InDimFilter implements DimFilter
   @Override
   public Filter toFilter()
   {
-    return new InFilter(dimension, ImmutableSet.copyOf(values), extractionFn);
+    return new InFilter(dimension, values, extractionFn);
   }
 
   @Override

processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java

@@ -22,9 +22,13 @@ package io.druid.query.filter;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
 import com.metamx.common.StringUtils;
 import io.druid.query.extraction.ExtractionFn;
 import io.druid.segment.filter.JavaScriptFilter;
+import org.mozilla.javascript.Context;
+import org.mozilla.javascript.Function;
+import org.mozilla.javascript.ScriptableObject;
 
 import java.nio.ByteBuffer;
 
@@ -34,6 +38,8 @@ public class JavaScriptDimFilter implements DimFilter
   private final String function;
   private final ExtractionFn extractionFn;
 
+  private final JavaScriptPredicate predicate;
+
   @JsonCreator
   public JavaScriptDimFilter(
       @JsonProperty("dimension") String dimension,
@@ -46,6 +52,7 @@ public class JavaScriptDimFilter implements DimFilter
     this.dimension = dimension;
     this.function = function;
     this.extractionFn = extractionFn;
+    this.predicate = new JavaScriptPredicate(function, extractionFn);
   }
 
   @JsonProperty
@@ -92,7 +99,7 @@ public class JavaScriptDimFilter implements DimFilter
   @Override
   public Filter toFilter()
   {
-    return new JavaScriptFilter(dimension, function, extractionFn);
+    return new JavaScriptFilter(dimension, predicate);
   }
 
   @Override
@@ -135,4 +142,76 @@ public class JavaScriptDimFilter implements DimFilter
     result = 31 * result + (extractionFn != null ? extractionFn.hashCode() : 0);
     return result;
   }
+
+  public static class JavaScriptPredicate implements Predicate<String>
+  {
+    final ScriptableObject scope;
+    final Function fnApply;
+    final String script;
+    final ExtractionFn extractionFn;
+
+    public JavaScriptPredicate(final String script, final ExtractionFn extractionFn)
+    {
+      Preconditions.checkNotNull(script, "script must not be null");
+      this.script = script;
+      this.extractionFn = extractionFn;
+
+      final Context cx = Context.enter();
+      try {
+        cx.setOptimizationLevel(9);
+        scope = cx.initStandardObjects();
+
+        fnApply = cx.compileFunction(scope, script, "script", 1, null);
+      }
+      finally {
+        Context.exit();
+      }
+    }
+
+    @Override
+    public boolean apply(final String input)
+    {
+      // one and only one context per thread
+      final Context cx = Context.enter();
+      try {
+        return applyInContext(cx, input);
+      }
+      finally {
+        Context.exit();
+      }
+    }
+
+    public boolean applyInContext(Context cx, String input)
+    {
+      if (extractionFn != null) {
+        input = extractionFn.apply(input);
+      }
+      return Context.toBoolean(fnApply.call(cx, scope, scope, new String[]{input}));
+    }
+
+    @Override
+    public boolean equals(Object o)
+    {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+
+      JavaScriptPredicate that = (JavaScriptPredicate) o;
+
+      if (!script.equals(that.script)) {
+        return false;
+      }
+
+      return true;
+    }
+
+    @Override
+    public int hashCode()
+    {
+      return script.hashCode();
+    }
+  }
 }

processing/src/main/java/io/druid/query/filter/RegexDimFilter.java

@@ -27,6 +27,7 @@ import io.druid.query.extraction.ExtractionFn;
 import io.druid.segment.filter.RegexFilter;
 
 import java.nio.ByteBuffer;
+import java.util.regex.Pattern;
 
 /**
  */
@@ -36,6 +37,8 @@ public class RegexDimFilter implements DimFilter
   private final String pattern;
   private final ExtractionFn extractionFn;
 
+  private final Pattern compiledPattern;
+
   @JsonCreator
   public RegexDimFilter(
       @JsonProperty("dimension") String dimension,
@@ -48,6 +51,7 @@ public class RegexDimFilter implements DimFilter
     this.dimension = dimension;
     this.pattern = pattern;
     this.extractionFn = extractionFn;
+    this.compiledPattern = Pattern.compile(pattern);
   }
 
   @JsonProperty
@@ -94,7 +98,7 @@ public class RegexDimFilter implements DimFilter
   @Override
   public Filter toFilter()
   {
-    return new RegexFilter(dimension, pattern, extractionFn);
+    return new RegexFilter(dimension, compiledPattern, extractionFn);
   }
 
   @Override

processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java

@@ -19,29 +19,27 @@
 
 package io.druid.segment.filter;
 
-import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.metamx.collections.bitmap.ImmutableBitmap;
-import io.druid.query.extraction.ExtractionFn;
 import io.druid.query.filter.BitmapIndexSelector;
 import io.druid.query.filter.Filter;
+import io.druid.query.filter.JavaScriptDimFilter;
 import io.druid.query.filter.ValueMatcher;
 import io.druid.query.filter.ValueMatcherFactory;
 import org.mozilla.javascript.Context;
-import org.mozilla.javascript.Function;
-import org.mozilla.javascript.ScriptableObject;
 
 public class JavaScriptFilter implements Filter
 {
-  private final JavaScriptPredicate predicate;
   private final String dimension;
-  private final ExtractionFn extractionFn;
+  private final JavaScriptDimFilter.JavaScriptPredicate predicate;
 
-  public JavaScriptFilter(String dimension, final String script, ExtractionFn extractionFn)
+  public JavaScriptFilter(
+      String dimension,
+      JavaScriptDimFilter.JavaScriptPredicate predicate
+  )
   {
     this.dimension = dimension;
-    this.predicate = new JavaScriptPredicate(script, extractionFn);
+    this.predicate = predicate;
-    this.extractionFn = extractionFn;
   }
 
   @Override
@@ -71,76 +69,4 @@ public class JavaScriptFilter implements Filter
     // suboptimal, since we need create one context per call to predicate.apply()
     return factory.makeValueMatcher(dimension, predicate);
   }
-
-  static class JavaScriptPredicate implements Predicate<String>
-  {
-    final ScriptableObject scope;
-    final Function fnApply;
-    final String script;
-    final ExtractionFn extractionFn;
-
-    public JavaScriptPredicate(final String script, final ExtractionFn extractionFn)
-    {
-      Preconditions.checkNotNull(script, "script must not be null");
-      this.script = script;
-      this.extractionFn = extractionFn;
-
-      final Context cx = Context.enter();
-      try {
-        cx.setOptimizationLevel(9);
-        scope = cx.initStandardObjects();
-
-        fnApply = cx.compileFunction(scope, script, "script", 1, null);
-      }
-      finally {
-        Context.exit();
-      }
-    }
-
-    @Override
-    public boolean apply(final String input)
-    {
-      // one and only one context per thread
-      final Context cx = Context.enter();
-      try {
-        return applyInContext(cx, input);
-      }
-      finally {
-        Context.exit();
-      }
-    }
-
-    public boolean applyInContext(Context cx, String input)
-    {
-      if (extractionFn != null) {
-        input = extractionFn.apply(input);
-      }
-      return Context.toBoolean(fnApply.call(cx, scope, scope, new String[]{input}));
-    }
-
-    @Override
-    public boolean equals(Object o)
-    {
-      if (this == o) {
-        return true;
-      }
-      if (o == null || getClass() != o.getClass()) {
-        return false;
-      }
-
-      JavaScriptPredicate that = (JavaScriptPredicate) o;
-
-      if (!script.equals(that.script)) {
-        return false;
-      }
-
-      return true;
-    }
-
-    @Override
-    public int hashCode()
-    {
-      return script.hashCode();
-    }
-  }
 }

processing/src/main/java/io/druid/segment/filter/RegexFilter.java

@@ -30,7 +30,7 @@ public class RegexFilter extends DimensionPredicateFilter
 {
   public RegexFilter(
       String dimension,
-      final String pattern,
+      final Pattern pattern,
       final ExtractionFn extractionFn
   )
   {
@@ -38,12 +38,10 @@ public class RegexFilter extends DimensionPredicateFilter
         dimension,
         new Predicate<String>()
         {
-          Pattern compiled = Pattern.compile(pattern);
-
           @Override
           public boolean apply(String input)
           {
-            return (input != null) && compiled.matcher(input).find();
+            return (input != null) && pattern.matcher(input).find();
           }
         },
         extractionFn

processing/src/test/java/io/druid/query/filter/InDimFilterSerDesrTest.java

@@ -36,7 +36,7 @@ public class InDimFilterSerDesrTest
 {
   private static ObjectMapper mapper;
 
-  private final String actualInFilter = "{\"type\":\"in\",\"dimension\":\"dimTest\",\"values\":[\"good\",\"bad\"],\"extractionFn\":null}";
+  private final String actualInFilter = "{\"type\":\"in\",\"dimension\":\"dimTest\",\"values\":[\"bad\",\"good\"],\"extractionFn\":null}";
   @Before
   public void setUp()
   {

processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java

@@ -27,31 +27,33 @@ import java.util.Arrays;
 
 public class JavaScriptDimFilterTest
 {
+  private static final String FN1 = "function(x) { return x }";
+  private static final String FN2 = "function(x) { return x + x }";
 
   @Test
   public void testGetCacheKey()
   {
-    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", "fn", null);
+    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null);
-    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null);
+    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", FN2, null);
     Assert.assertFalse(Arrays.equals(javaScriptDimFilter.getCacheKey(), javaScriptDimFilter2.getCacheKey()));
 
     RegexDimExtractionFn regexFn = new RegexDimExtractionFn(".*", false, null);
-    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("dim", "fn", regexFn);
+    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("dim", FN1, regexFn);
     Assert.assertFalse(Arrays.equals(javaScriptDimFilter.getCacheKey(), javaScriptDimFilter3.getCacheKey()));
   }
 
   @Test
   public void testEquals()
   {
-    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", "fn", null);
+    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null);
-    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null);
+    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", FN2, null);
-    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", "mfn", null);
+    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", FN2, null);
     Assert.assertNotEquals(javaScriptDimFilter, javaScriptDimFilter2);
     Assert.assertEquals(javaScriptDimFilter2, javaScriptDimFilter3);
 
     RegexDimExtractionFn regexFn = new RegexDimExtractionFn(".*", false, null);
-    JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", "fn", regexFn);
+    JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", FN1, regexFn);
-    JavaScriptDimFilter javaScriptDimFilter5 = new JavaScriptDimFilter("dim", "fn", regexFn);
+    JavaScriptDimFilter javaScriptDimFilter5 = new JavaScriptDimFilter("dim", FN1, regexFn);
     Assert.assertNotEquals(javaScriptDimFilter, javaScriptDimFilter3);
     Assert.assertEquals(javaScriptDimFilter4, javaScriptDimFilter5);
   }
@@ -59,15 +61,15 @@ public class JavaScriptDimFilterTest
   @Test
   public void testHashcode()
   {
-    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", "fn", null);
+    JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null);
-    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null);
+    JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", FN2, null);
-    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", "mfn", null);
+    JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", FN2, null);
     Assert.assertNotEquals(javaScriptDimFilter.hashCode(), javaScriptDimFilter2.hashCode());
     Assert.assertEquals(javaScriptDimFilter2.hashCode(), javaScriptDimFilter3.hashCode());
 
     RegexDimExtractionFn regexFn = new RegexDimExtractionFn(".*", false, null);
-    JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", "fn", regexFn);
+    JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", FN1, regexFn);
-    JavaScriptDimFilter javaScriptDimFilter5 = new JavaScriptDimFilter("dim", "fn", regexFn);
+    JavaScriptDimFilter javaScriptDimFilter5 = new JavaScriptDimFilter("dim", FN1, regexFn);
     Assert.assertNotEquals(javaScriptDimFilter.hashCode(), javaScriptDimFilter3.hashCode());
     Assert.assertEquals(javaScriptDimFilter4.hashCode(), javaScriptDimFilter5.hashCode());
   }

processing/src/test/java/io/druid/segment/filter/JavaScriptFilterTest.java

@@ -48,7 +48,7 @@ import java.util.List;
 import java.util.Map;
 
 @RunWith(Parameterized.class)
-public class JavascriptFilterTest extends BaseFilterTest
+public class JavaScriptFilterTest extends BaseFilterTest
 {
   private static final String TIMESTAMP_COLUMN = "timestamp";
 
@@ -72,7 +72,7 @@ public class JavascriptFilterTest extends BaseFilterTest
       PARSER.parse(ImmutableMap.<String, Object>of("dim0", "5", "dim1", "abc"))
   );
 
-  public JavascriptFilterTest(
+  public JavaScriptFilterTest(
       String testName,
       IndexBuilder indexBuilder,
       Function<IndexBuilder, Pair<StorageAdapter, Closeable>> finisher,