From 105cdebb2c2d52647fa4a64967c74f08b39f8784 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 29 Mar 2022 18:44:32 -0700 Subject: [PATCH] Use Set instead of LongSet in long script field (#85417) The long script field uses hppc's LongSet to store the unique values to be queried. However, this set should be relatively small. This commit changes the internals of the runtime long script field to use Set. relates #84735 --- .../index/mapper/DateScriptFieldType.java | 7 +++---- .../index/mapper/LongScriptFieldType.java | 7 +++---- .../runtime/LongScriptFieldTermsQuery.java | 9 ++++----- .../org/elasticsearch/common/NumbersTests.java | 17 +++++++++-------- .../runtime/LongScriptFieldTermsQueryTests.java | 14 +++++++------- 5 files changed, 26 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java index 2f83db6dc55e5..c442d6c498b00 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java @@ -8,9 +8,6 @@ package org.elasticsearch.index.mapper; -import com.carrotsearch.hppc.LongHashSet; -import com.carrotsearch.hppc.LongSet; - import org.apache.lucene.search.Query; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.time.DateFormatter; @@ -38,9 +35,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; @@ -233,7 +232,7 @@ public Query termsQuery(Collection values, SearchExecutionContext context) { return Queries.newMatchAllQuery(); } return DateFieldType.handleNow(context, now -> { - LongSet terms = new LongHashSet(values.size()); + Set terms = new HashSet<>(values.size()); for (Object value : values) { terms.add(DateFieldType.parseToLong(value, false, null, this.dateMathParser, now, DateFieldMapper.Resolution.MILLISECONDS)); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java index dfbe79176ae4d..262fe1bcbe4ae 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java @@ -8,9 +8,6 @@ package org.elasticsearch.index.mapper; -import com.carrotsearch.hppc.LongHashSet; -import com.carrotsearch.hppc.LongSet; - import org.apache.lucene.search.Query; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.time.DateMathParser; @@ -30,7 +27,9 @@ import java.time.ZoneId; import java.util.Collection; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; @@ -137,7 +136,7 @@ public Query termsQuery(Collection values, SearchExecutionContext context) { if (values.isEmpty()) { return Queries.newMatchAllQuery(); } - LongSet terms = new LongHashSet(values.size()); + Set terms = new HashSet<>(values.size()); for (Object value : values) { if (NumberType.hasDecimalPart(value)) { continue; diff --git a/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java index 59ed6591522a7..3819f4fbd95a3 100644 --- a/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java +++ b/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java @@ -8,23 +8,22 @@ package org.elasticsearch.search.runtime; -import com.carrotsearch.hppc.LongSet; - import org.apache.lucene.index.LeafReaderContext; import org.elasticsearch.script.AbstractLongFieldScript; import org.elasticsearch.script.Script; import java.util.Objects; +import java.util.Set; import java.util.function.Function; public class LongScriptFieldTermsQuery extends AbstractLongScriptFieldQuery { - private final LongSet terms; + private final Set terms; public LongScriptFieldTermsQuery( Script script, Function leafFactory, String fieldName, - LongSet terms + Set terms ) { super(script, leafFactory, fieldName); this.terms = terms; @@ -62,7 +61,7 @@ public boolean equals(Object obj) { return terms.equals(other.terms); } - LongSet terms() { + Set terms() { return terms; } } diff --git a/server/src/test/java/org/elasticsearch/common/NumbersTests.java b/server/src/test/java/org/elasticsearch/common/NumbersTests.java index 6be3fc23ef2dc..9a4e3caf2a2ff 100644 --- a/server/src/test/java/org/elasticsearch/common/NumbersTests.java +++ b/server/src/test/java/org/elasticsearch/common/NumbersTests.java @@ -16,20 +16,21 @@ import java.math.BigInteger; import java.util.concurrent.atomic.AtomicInteger; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; public class NumbersTests extends ESTestCase { @Timeout(millis = 10000) public void testToLong() { - assertEquals(3L, Numbers.toLong("3", false)); - assertEquals(3L, Numbers.toLong("3.1", true)); - assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", false)); - assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", false)); - assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", true)); - assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", true)); - assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.99", true)); - assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.99", true)); + assertThat(Numbers.toLong("3", false), equalTo(3L)); + assertThat(Numbers.toLong("3.1", true), equalTo(3L)); + assertThat(Numbers.toLong("9223372036854775807.00", false), equalTo(9223372036854775807L)); + assertThat(Numbers.toLong("-9223372036854775808.00", false), equalTo(-9223372036854775808L)); + assertThat(Numbers.toLong("9223372036854775807.00", true), equalTo(9223372036854775807L)); + assertThat(Numbers.toLong("-9223372036854775808.00", true), equalTo(-9223372036854775808L)); + assertThat(Numbers.toLong("9223372036854775807.99", true), equalTo(9223372036854775807L)); + assertThat(Numbers.toLong("-9223372036854775808.99", true), equalTo(-9223372036854775808L)); assertEquals( "Value [9223372036854775808] is out of range for a long", diff --git a/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java b/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java index fd8462f140568..e5d272488da3d 100644 --- a/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java @@ -8,17 +8,17 @@ package org.elasticsearch.search.runtime; -import com.carrotsearch.hppc.LongHashSet; -import com.carrotsearch.hppc.LongSet; - import org.elasticsearch.script.Script; +import java.util.HashSet; +import java.util.Set; + import static org.hamcrest.Matchers.equalTo; public class LongScriptFieldTermsQueryTests extends AbstractLongScriptFieldQueryTestCase { @Override protected LongScriptFieldTermsQuery createTestInstance() { - LongSet terms = new LongHashSet(); + Set terms = new HashSet<>(); int count = between(1, 100); while (terms.size() < count) { terms.add(randomLong()); @@ -35,12 +35,12 @@ protected LongScriptFieldTermsQuery copy(LongScriptFieldTermsQuery orig) { protected LongScriptFieldTermsQuery mutate(LongScriptFieldTermsQuery orig) { Script script = orig.script(); String fieldName = orig.fieldName(); - LongSet terms = orig.terms(); + Set terms = orig.terms(); switch (randomInt(2)) { case 0 -> script = randomValueOtherThan(script, this::randomScript); case 1 -> fieldName += "modified"; case 2 -> { - terms = new LongHashSet(terms); + terms = new HashSet<>(terms); while (false == terms.add(randomLong())) { // Random long was already in the set } @@ -52,7 +52,7 @@ protected LongScriptFieldTermsQuery mutate(LongScriptFieldTermsQuery orig) { @Override public void testMatches() { - LongScriptFieldTermsQuery query = new LongScriptFieldTermsQuery(randomScript(), leafFactory, "test", LongHashSet.from(1, 2, 3)); + LongScriptFieldTermsQuery query = new LongScriptFieldTermsQuery(randomScript(), leafFactory, "test", Set.of(1L, 2L, 3L)); assertTrue(query.matches(new long[] { 1 }, 1)); assertTrue(query.matches(new long[] { 2 }, 1)); assertTrue(query.matches(new long[] { 3 }, 1));