Browse Source

Remove hppc from dfs search (#84688)

This commit removes the HppcMaps class and converts the few existing
uses, in DFS code, to use Map.
Ryan Ernst 3 years ago
parent
commit
df5dbd5a04

+ 21 - 27
server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java

@@ -9,7 +9,6 @@
 package org.elasticsearch.action.search;
 
 import com.carrotsearch.hppc.IntArrayList;
-import com.carrotsearch.hppc.ObjectObjectHashMap;
 
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.CollectionStatistics;
@@ -23,7 +22,6 @@ import org.apache.lucene.search.TopFieldDocs;
 import org.apache.lucene.search.TotalHits;
 import org.apache.lucene.search.TotalHits.Relation;
 import org.elasticsearch.common.breaker.CircuitBreaker;
-import org.elasticsearch.common.collect.HppcMaps;
 import org.elasticsearch.common.lucene.search.TopDocsAndMaxScore;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.lucene.grouping.TopFieldGroups;
@@ -73,8 +71,8 @@ public final class SearchPhaseController {
     }
 
     public AggregatedDfs aggregateDfs(Collection<DfsSearchResult> results) {
-        ObjectObjectHashMap<Term, TermStatistics> termStatistics = HppcMaps.newNoNullKeysMap();
-        ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics = HppcMaps.newNoNullKeysMap();
+        Map<Term, TermStatistics> termStatistics = new HashMap<>();
+        Map<String, CollectionStatistics> fieldStatistics = new HashMap<>();
         long aggMaxDoc = 0;
         for (DfsSearchResult lEntry : results) {
             final Term[] terms = lEntry.terms();
@@ -103,29 +101,25 @@ public final class SearchPhaseController {
             }
 
             assert lEntry.fieldStatistics().containsKey(null) == false;
-            final Object[] keys = lEntry.fieldStatistics().keys;
-            final Object[] values = lEntry.fieldStatistics().values;
-            for (int i = 0; i < keys.length; i++) {
-                if (keys[i] != null) {
-                    String key = (String) keys[i];
-                    CollectionStatistics value = (CollectionStatistics) values[i];
-                    if (value == null) {
-                        continue;
-                    }
-                    assert key != null;
-                    CollectionStatistics existing = fieldStatistics.get(key);
-                    if (existing != null) {
-                        CollectionStatistics merged = new CollectionStatistics(
-                            key,
-                            existing.maxDoc() + value.maxDoc(),
-                            existing.docCount() + value.docCount(),
-                            existing.sumTotalTermFreq() + value.sumTotalTermFreq(),
-                            existing.sumDocFreq() + value.sumDocFreq()
-                        );
-                        fieldStatistics.put(key, merged);
-                    } else {
-                        fieldStatistics.put(key, value);
-                    }
+            for (var entry : lEntry.fieldStatistics().entrySet()) {
+                String key = entry.getKey();
+                CollectionStatistics value = entry.getValue();
+                if (value == null) {
+                    continue;
+                }
+                assert key != null;
+                CollectionStatistics existing = fieldStatistics.get(key);
+                if (existing != null) {
+                    CollectionStatistics merged = new CollectionStatistics(
+                        key,
+                        existing.maxDoc() + value.maxDoc(),
+                        existing.docCount() + value.docCount(),
+                        existing.sumTotalTermFreq() + value.sumTotalTermFreq(),
+                        existing.sumDocFreq() + value.sumDocFreq()
+                    );
+                    fieldStatistics.put(key, merged);
+                } else {
+                    fieldStatistics.put(key, value);
                 }
             }
             aggMaxDoc += lEntry.maxDoc();

+ 0 - 118
server/src/main/java/org/elasticsearch/common/collect/HppcMaps.java

@@ -1,118 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License
- * 2.0 and the Server Side Public License, v 1; you may not use this file except
- * in compliance with, at your election, the Elastic License 2.0 or the Server
- * Side Public License, v 1.
- */
-
-package org.elasticsearch.common.collect;
-
-import com.carrotsearch.hppc.ObjectIntHashMap;
-import com.carrotsearch.hppc.ObjectLookupContainer;
-import com.carrotsearch.hppc.ObjectObjectHashMap;
-import com.carrotsearch.hppc.cursors.ObjectCursor;
-
-import java.util.Iterator;
-
-public final class HppcMaps {
-
-    private HppcMaps() {}
-
-    /**
-     * Returns a new map with the given number of expected elements.
-     *
-     * @param expectedElements
-     *          The expected number of elements guaranteed not to cause buffer
-     *          expansion (inclusive).
-     */
-    public static <K, V> ObjectObjectHashMap<K, V> newMap(int expectedElements) {
-        return new ObjectObjectHashMap<>(expectedElements);
-    }
-
-    /**
-     * Returns a map like {@link #newMap} that does not accept <code>null</code> keys
-     */
-    public static <K, V> ObjectObjectHashMap<K, V> newNoNullKeysMap() {
-        return newNoNullKeysMap(16);
-    }
-
-    /**
-     * Returns a map like {@link #newMap(int)} that does not accept <code>null</code> keys
-     *
-     * @param expectedElements
-     *          The expected number of elements guaranteed not to cause buffer
-     *          expansion (inclusive).
-     */
-    public static <K, V> ObjectObjectHashMap<K, V> newNoNullKeysMap(int expectedElements) {
-        return new ObjectObjectHashMap<K, V>(expectedElements) {
-            @Override
-            public V put(K key, V value) {
-                if (key == null) {
-                    throw new IllegalArgumentException("Map key must not be null");
-                }
-                return super.put(key, value);
-            }
-        };
-    }
-
-    /**
-     * @return an intersection view over the two specified containers (which can be KeyContainer or ObjectHashSet).
-     */
-    // Hppc has forEach, but this means we need to build an intermediate set, with this method we just iterate
-    // over each unique value without creating a third set.
-    public static <T> Iterable<T> intersection(ObjectLookupContainer<T> container1, final ObjectLookupContainer<T> container2) {
-        assert container1 != null && container2 != null;
-        final Iterator<ObjectCursor<T>> iterator = container1.iterator();
-        final Iterator<T> intersection = new Iterator<T>() {
-
-            T current;
-
-            @Override
-            public boolean hasNext() {
-                if (iterator.hasNext()) {
-                    do {
-                        T next = iterator.next().value;
-                        if (container2.contains(next)) {
-                            current = next;
-                            return true;
-                        }
-                    } while (iterator.hasNext());
-                }
-                return false;
-            }
-
-            @Override
-            public T next() {
-                return current;
-            }
-
-            @Override
-            public void remove() {
-                throw new UnsupportedOperationException();
-            }
-        };
-        return new Iterable<T>() {
-            @Override
-            public Iterator<T> iterator() {
-                return intersection;
-            }
-        };
-    }
-
-    public static final class Object {
-        public static final class Integer {
-            public static <V> ObjectIntHashMap<V> ensureNoNullKeys(int capacity, float loadFactor) {
-                return new ObjectIntHashMap<V>(capacity, loadFactor) {
-                    @Override
-                    public int put(V key, int value) {
-                        if (key == null) {
-                            throw new IllegalArgumentException("Map key must not be null");
-                        }
-                        return super.put(key, value);
-                    }
-                };
-            }
-        }
-    }
-}

+ 11 - 17
server/src/main/java/org/elasticsearch/search/dfs/AggregatedDfs.java

@@ -8,28 +8,26 @@
 
 package org.elasticsearch.search.dfs;
 
-import com.carrotsearch.hppc.ObjectObjectHashMap;
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
-
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.TermStatistics;
-import org.elasticsearch.common.collect.HppcMaps;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 public class AggregatedDfs implements Writeable {
 
-    private ObjectObjectHashMap<Term, TermStatistics> termStatistics;
-    private ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics;
+    private Map<Term, TermStatistics> termStatistics;
+    private Map<String, CollectionStatistics> fieldStatistics;
     private long maxDoc;
 
     public AggregatedDfs(StreamInput in) throws IOException {
         int size = in.readVInt();
-        termStatistics = HppcMaps.newMap(size);
+        termStatistics = new HashMap<>(size);
         for (int i = 0; i < size; i++) {
             Term term = new Term(in.readString(), in.readBytesRef());
             TermStatistics stats = new TermStatistics(in.readBytesRef(), in.readVLong(), DfsSearchResult.subOne(in.readVLong()));
@@ -39,21 +37,17 @@ public class AggregatedDfs implements Writeable {
         maxDoc = in.readVLong();
     }
 
-    public AggregatedDfs(
-        ObjectObjectHashMap<Term, TermStatistics> termStatistics,
-        ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics,
-        long maxDoc
-    ) {
+    public AggregatedDfs(Map<Term, TermStatistics> termStatistics, Map<String, CollectionStatistics> fieldStatistics, long maxDoc) {
         this.termStatistics = termStatistics;
         this.fieldStatistics = fieldStatistics;
         this.maxDoc = maxDoc;
     }
 
-    public ObjectObjectHashMap<Term, TermStatistics> termStatistics() {
+    public Map<Term, TermStatistics> termStatistics() {
         return termStatistics;
     }
 
-    public ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics() {
+    public Map<String, CollectionStatistics> fieldStatistics() {
         return fieldStatistics;
     }
 
@@ -65,11 +59,11 @@ public class AggregatedDfs implements Writeable {
     public void writeTo(final StreamOutput out) throws IOException {
         out.writeVInt(termStatistics.size());
 
-        for (ObjectObjectCursor<Term, TermStatistics> c : termStatistics()) {
-            Term term = c.key;
+        for (var entry : termStatistics().entrySet()) {
+            Term term = entry.getKey();
             out.writeString(term.field());
             out.writeBytesRef(term.bytes());
-            TermStatistics stats = c.value;
+            TermStatistics stats = entry.getValue();
             out.writeBytesRef(stats.term());
             out.writeVLong(stats.docFreq());
             out.writeVLong(DfsSearchResult.addOne(stats.totalTermFreq()));

+ 1 - 4
server/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java

@@ -8,15 +8,12 @@
 
 package org.elasticsearch.search.dfs;
 
-import com.carrotsearch.hppc.ObjectObjectHashMap;
-
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.ScoreMode;
 import org.apache.lucene.search.TermStatistics;
-import org.elasticsearch.common.collect.HppcMaps;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.rescore.RescoreContext;
 import org.elasticsearch.tasks.TaskCancelledException;
@@ -33,7 +30,7 @@ public class DfsPhase {
 
     public void execute(SearchContext context) {
         try {
-            ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics = HppcMaps.newNoNullKeysMap();
+            Map<String, CollectionStatistics> fieldStatistics = new HashMap<>();
             Map<Term, TermStatistics> stats = new HashMap<>();
             IndexSearcher searcher = new IndexSearcher(context.searcher().getIndexReader()) {
                 @Override

+ 11 - 14
server/src/main/java/org/elasticsearch/search/dfs/DfsSearchResult.java

@@ -8,15 +8,11 @@
 
 package org.elasticsearch.search.dfs;
 
-import com.carrotsearch.hppc.ObjectObjectHashMap;
-import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
-
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.TermStatistics;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
-import org.elasticsearch.common.collect.HppcMaps;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.search.SearchPhaseResult;
@@ -25,6 +21,8 @@ import org.elasticsearch.search.internal.ShardSearchContextId;
 import org.elasticsearch.search.internal.ShardSearchRequest;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 public class DfsSearchResult extends SearchPhaseResult {
 
@@ -32,7 +30,7 @@ public class DfsSearchResult extends SearchPhaseResult {
     private static final TermStatistics[] EMPTY_TERM_STATS = new TermStatistics[0];
     private Term[] terms;
     private TermStatistics[] termStatistics;
-    private ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics = HppcMaps.newNoNullKeysMap();
+    private Map<String, CollectionStatistics> fieldStatistics = new HashMap<>();
     private int maxDoc;
 
     public DfsSearchResult(StreamInput in) throws IOException {
@@ -77,7 +75,7 @@ public class DfsSearchResult extends SearchPhaseResult {
         return this;
     }
 
-    public DfsSearchResult fieldStatistics(ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics) {
+    public DfsSearchResult fieldStatistics(Map<String, CollectionStatistics> fieldStatistics) {
         this.fieldStatistics = fieldStatistics;
         return this;
     }
@@ -90,7 +88,7 @@ public class DfsSearchResult extends SearchPhaseResult {
         return termStatistics;
     }
 
-    public ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics() {
+    public Map<String, CollectionStatistics> fieldStatistics() {
         return fieldStatistics;
     }
 
@@ -110,13 +108,12 @@ public class DfsSearchResult extends SearchPhaseResult {
         }
     }
 
-    public static void writeFieldStats(StreamOutput out, ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics)
-        throws IOException {
+    public static void writeFieldStats(StreamOutput out, Map<String, CollectionStatistics> fieldStatistics) throws IOException {
         out.writeVInt(fieldStatistics.size());
 
-        for (ObjectObjectCursor<String, CollectionStatistics> c : fieldStatistics) {
-            out.writeString(c.key);
-            CollectionStatistics statistics = c.value;
+        for (var entry : fieldStatistics.entrySet()) {
+            out.writeString(entry.getKey());
+            CollectionStatistics statistics = entry.getValue();
             assert statistics.maxDoc() >= 0;
             out.writeVLong(statistics.maxDoc());
             // stats are always positive numbers
@@ -144,9 +141,9 @@ public class DfsSearchResult extends SearchPhaseResult {
         }
     }
 
-    static ObjectObjectHashMap<String, CollectionStatistics> readFieldStats(StreamInput in) throws IOException {
+    static Map<String, CollectionStatistics> readFieldStats(StreamInput in) throws IOException {
         final int numFieldStatistics = in.readVInt();
-        ObjectObjectHashMap<String, CollectionStatistics> fieldStatistics = HppcMaps.newNoNullKeysMap(numFieldStatistics);
+        Map<String, CollectionStatistics> fieldStatistics = new HashMap<>(numFieldStatistics);
         for (int i = 0; i < numFieldStatistics; i++) {
             final String field = in.readString();
             assert field != null;

+ 0 - 88
server/src/test/java/org/elasticsearch/common/hppc/HppcMapsTests.java

@@ -1,88 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License
- * 2.0 and the Server Side Public License, v 1; you may not use this file except
- * in compliance with, at your election, the Elastic License 2.0 or the Server
- * Side Public License, v 1.
- */
-package org.elasticsearch.common.hppc;
-
-import com.carrotsearch.hppc.ObjectHashSet;
-
-import org.elasticsearch.Assertions;
-import org.elasticsearch.common.collect.HppcMaps;
-import org.elasticsearch.test.ESTestCase;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import static org.hamcrest.Matchers.equalTo;
-
-public class HppcMapsTests extends ESTestCase {
-    public void testIntersection() throws Exception {
-        assumeTrue("assertions enabled", Assertions.ENABLED);
-        ObjectHashSet<String> set1 = ObjectHashSet.from("1", "2", "3");
-        ObjectHashSet<String> set2 = ObjectHashSet.from("1", "2", "3");
-        List<String> values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(3));
-        assertThat(values.contains("1"), equalTo(true));
-        assertThat(values.contains("2"), equalTo(true));
-        assertThat(values.contains("3"), equalTo(true));
-
-        set1 = ObjectHashSet.from("1", "2", "3");
-        set2 = ObjectHashSet.from("3", "4", "5");
-        values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(1));
-        assertThat(values.get(0), equalTo("3"));
-
-        set1 = ObjectHashSet.from("1", "2", "3");
-        set2 = ObjectHashSet.from("4", "5", "6");
-        values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(0));
-
-        set1 = ObjectHashSet.from();
-        set2 = ObjectHashSet.from("3", "4", "5");
-        values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(0));
-
-        set1 = ObjectHashSet.from("1", "2", "3");
-        set2 = ObjectHashSet.from();
-        values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(0));
-
-        set1 = ObjectHashSet.from();
-        set2 = ObjectHashSet.from();
-        values = toList(HppcMaps.intersection(set1, set2));
-        assertThat(values.size(), equalTo(0));
-
-        set1 = null;
-        set2 = ObjectHashSet.from();
-        try {
-            toList(HppcMaps.intersection(set1, set2));
-            fail();
-        } catch (AssertionError e) {}
-
-        set1 = ObjectHashSet.from();
-        set2 = null;
-        try {
-            toList(HppcMaps.intersection(set1, set2));
-            fail();
-        } catch (AssertionError e) {}
-
-        set1 = null;
-        set2 = null;
-        try {
-            toList(HppcMaps.intersection(set1, set2));
-            fail();
-        } catch (AssertionError e) {}
-    }
-
-    private List<String> toList(Iterable<String> iterable) {
-        List<String> list = new ArrayList<>();
-        for (String s : iterable) {
-            list.add(s);
-        }
-        return list;
-    }
-
-}