From 7d3e55717d27752a2674a90da0c374ba8ac5c558 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 20 Apr 2016 13:28:47 -0700 Subject: [PATCH] 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. --- .../io/druid/query/filter/InDimFilter.java | 22 +++-- .../query/filter/JavaScriptDimFilter.java | 81 ++++++++++++++++- .../io/druid/query/filter/RegexDimFilter.java | 6 +- .../segment/filter/JavaScriptFilter.java | 88 ++----------------- .../io/druid/segment/filter/RegexFilter.java | 6 +- .../query/filter/InDimFilterSerDesrTest.java | 2 +- .../query/filter/JavaScriptDimFilterTest.java | 28 +++--- ...terTest.java => JavaScriptFilterTest.java} | 4 +- 8 files changed, 122 insertions(+), 115 deletions(-) rename processing/src/test/java/io/druid/segment/filter/{JavascriptFilterTest.java => JavaScriptFilterTest.java} (98%) diff --git a/processing/src/main/java/io/druid/query/filter/InDimFilter.java b/processing/src/main/java/io/druid/query/filter/InDimFilter.java index 2b189919540..f6d3861ac9a 100644 --- a/processing/src/main/java/io/druid/query/filter/InDimFilter.java +++ b/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 values; + private final ImmutableSortedSet 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() { @@ -78,7 +73,7 @@ public class InDimFilter implements DimFilter } @JsonProperty - public List getValues() + public Set 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 diff --git a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java index 9509b8f9445..634d75bb154 100644 --- a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java +++ b/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 + { + 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(); + } + } } diff --git a/processing/src/main/java/io/druid/query/filter/RegexDimFilter.java b/processing/src/main/java/io/druid/query/filter/RegexDimFilter.java index 8b182b5d5e8..da6e49f3309 100644 --- a/processing/src/main/java/io/druid/query/filter/RegexDimFilter.java +++ b/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 diff --git a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java index ab0944323c7..9f319db3b67 100644 --- a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java +++ b/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.extractionFn = extractionFn; + this.predicate = predicate; } @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 - { - 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(); - } - } } diff --git a/processing/src/main/java/io/druid/segment/filter/RegexFilter.java b/processing/src/main/java/io/druid/segment/filter/RegexFilter.java index 02a267e0597..327f1ba118f 100644 --- a/processing/src/main/java/io/druid/segment/filter/RegexFilter.java +++ b/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() { - 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 diff --git a/processing/src/test/java/io/druid/query/filter/InDimFilterSerDesrTest.java b/processing/src/test/java/io/druid/query/filter/InDimFilterSerDesrTest.java index 7e7539fe73d..34b5ea7099e 100644 --- a/processing/src/test/java/io/druid/query/filter/InDimFilterSerDesrTest.java +++ b/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() { diff --git a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java index 3f8cbd84ce9..d13b6e14f03 100644 --- a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java +++ b/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 javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null); + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, 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 javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null); - JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", "mfn", null); + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null); + JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", FN2, 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 javaScriptDimFilter5 = new JavaScriptDimFilter("dim", "fn", regexFn); + JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", FN1, 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 javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn", null); - JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter("di", "mfn", null); + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null); + JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", FN2, 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 javaScriptDimFilter5 = new JavaScriptDimFilter("dim", "fn", regexFn); + JavaScriptDimFilter javaScriptDimFilter4 = new JavaScriptDimFilter("dim", FN1, regexFn); + JavaScriptDimFilter javaScriptDimFilter5 = new JavaScriptDimFilter("dim", FN1, regexFn); Assert.assertNotEquals(javaScriptDimFilter.hashCode(), javaScriptDimFilter3.hashCode()); Assert.assertEquals(javaScriptDimFilter4.hashCode(), javaScriptDimFilter5.hashCode()); } diff --git a/processing/src/test/java/io/druid/segment/filter/JavascriptFilterTest.java b/processing/src/test/java/io/druid/segment/filter/JavaScriptFilterTest.java similarity index 98% rename from processing/src/test/java/io/druid/segment/filter/JavascriptFilterTest.java rename to processing/src/test/java/io/druid/segment/filter/JavaScriptFilterTest.java index 3bc471c42f3..04af6051f11 100644 --- a/processing/src/test/java/io/druid/segment/filter/JavascriptFilterTest.java +++ b/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.of("dim0", "5", "dim1", "abc")) ); - public JavascriptFilterTest( + public JavaScriptFilterTest( String testName, IndexBuilder indexBuilder, Function> finisher,