Skip to content

Commit

Permalink
Use Set instead of LongSet in double script field
Browse files Browse the repository at this point in the history 
The double 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 double script field to use Set.

relates elastic#84735
  • Loading branch information
@rjernst
rjernst committed Mar 30, 2022
1 parent 105cdeb commit 7d65c47
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 19 deletions.
7 changes: 3 additions & 4 deletions server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -139,7 +138,7 @@ public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
if (values.isEmpty()) {
return Queries.newMatchAllQuery();
}
LongSet terms = new LongHashSet(values.size());
Set<Long> terms = new HashSet<>(values.size());
for (Object value : values) {
terms.add(Double.doubleToLongBits(NumberType.objectToDouble(value)));
}
Expand Down
12 changes: 5 additions & 7 deletions server/src/main/java/org/elasticsearch/search/runtime/DoubleScriptFieldTermsQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,21 @@

package org.elasticsearch.search.runtime;

import com.carrotsearch.hppc.LongSet;
import com.carrotsearch.hppc.cursors.LongCursor;

import org.elasticsearch.script.DoubleFieldScript;
import org.elasticsearch.script.Script;

import java.util.Arrays;
import java.util.Objects;
import java.util.Set;

public class DoubleScriptFieldTermsQuery extends AbstractDoubleScriptFieldQuery {
private final LongSet terms;
private final Set<Long> terms;

/**
* Build the query.
* @param terms The terms converted to a long with {@link Double#doubleToLongBits(double)}.
*/
public DoubleScriptFieldTermsQuery(Script script, DoubleFieldScript.LeafFactory leafFactory, String fieldName, LongSet terms) {
public DoubleScriptFieldTermsQuery(Script script, DoubleFieldScript.LeafFactory leafFactory, String fieldName, Set<Long> terms) {
super(script, leafFactory, fieldName);
this.terms = terms;
}
Expand Down Expand Up @@ -66,8 +64,8 @@ public boolean equals(Object obj) {
double[] terms() {
double[] result = new double[terms.size()];
int i = 0;
for (LongCursor lc : terms) {
result[i++] = Double.longBitsToDouble(lc.value);
for (long l : terms) {
result[i++] = Double.longBitsToDouble(l);
}
return result;
}
Expand Down
15 changes: 7 additions & 8 deletions server/src/test/java/org/elasticsearch/search/runtime/DoubleScriptFieldTermsQueryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@

package org.elasticsearch.search.runtime;

import com.carrotsearch.hppc.LongHashSet;
import com.carrotsearch.hppc.LongSet;

import org.elasticsearch.script.Script;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static java.util.stream.Collectors.toList;
import static org.hamcrest.Matchers.endsWith;
Expand All @@ -25,7 +24,7 @@
public class DoubleScriptFieldTermsQueryTests extends AbstractDoubleScriptFieldQueryTestCase<DoubleScriptFieldTermsQuery> {
@Override
protected DoubleScriptFieldTermsQuery createTestInstance() {
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
int count = between(1, 100);
while (terms.size() < count) {
terms.add(Double.doubleToLongBits(randomDouble()));
Expand All @@ -35,7 +34,7 @@ protected DoubleScriptFieldTermsQuery createTestInstance() {

@Override
protected DoubleScriptFieldTermsQuery copy(DoubleScriptFieldTermsQuery orig) {
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
for (double term : orig.terms()) {
terms.add(Double.doubleToLongBits(term));
}
Expand All @@ -46,15 +45,15 @@ protected DoubleScriptFieldTermsQuery copy(DoubleScriptFieldTermsQuery orig) {
protected DoubleScriptFieldTermsQuery mutate(DoubleScriptFieldTermsQuery orig) {
Script script = orig.script();
String fieldName = orig.fieldName();
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
for (double term : orig.terms()) {
terms.add(Double.doubleToLongBits(term));
}
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(Double.doubleToLongBits(randomDouble()))) {
// Random double was already in the set
}
Expand All @@ -70,7 +69,7 @@ public void testMatches() {
randomScript(),
leafFactory,
"test",
LongHashSet.from(Double.doubleToLongBits(0.1), Double.doubleToLongBits(0.2), Double.doubleToLongBits(7.5))
Set.of(Double.doubleToLongBits(0.1), Double.doubleToLongBits(0.2), Double.doubleToLongBits(7.5))
);
assertTrue(query.matches(new double[] { 0.1 }, 1));
assertTrue(query.matches(new double[] { 0.2 }, 1));
Expand Down

0 comments on commit 7d65c47

Please sign in to comment.