From 97e40080029ec6f0d192e41e591d377d39567903 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Fri, 11 Dec 2020 18:18:29 +0200 Subject: [PATCH 1/2] EQL: Optimize string retaintion When iterating across search hits, common strings such as the index name or common keys get allocated new strings. When dealing with a large number of potential keys these add up and end up wasting memory though their content is the same. This commit introduces a simple LRU cache (up to 64 entries) to minimize the duplication. --- .../eql/execution/assembler/Criterion.java | 15 ++----- .../eql/execution/search/HitReference.java | 16 ++++--- .../eql/execution/sequence/SequenceKey.java | 2 +- .../execution/sequence/TumblingWindow.java | 43 ++++++++++++++++++- 4 files changed, 56 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java index 1897324feeac5..4fc1324bdf796 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java @@ -10,7 +10,6 @@ import org.elasticsearch.xpack.eql.EqlIllegalArgumentException; import org.elasticsearch.xpack.eql.execution.search.Ordinal; import org.elasticsearch.xpack.eql.execution.search.QueryRequest; -import org.elasticsearch.xpack.eql.execution.sequence.SequenceKey; import org.elasticsearch.xpack.ql.execution.search.extractor.HitExtractor; import java.util.List; @@ -52,20 +51,14 @@ public Q queryRequest() { return queryRequest; } - public int keySize() { - return keys.size(); - } - - public SequenceKey key(SearchHit hit) { - SequenceKey key; - if (keys.isEmpty()) { - key = SequenceKey.NONE; - } else { + public Object[] key(SearchHit hit) { + Object[] key = null; + if (keys.isEmpty() == false) { Object[] docKeys = new Object[keys.size()]; for (int i = 0; i < docKeys.length; i++) { docKeys[i] = keys.get(i).extract(hit); } - key = new SequenceKey(docKeys); + key = docKeys; } return key; } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/HitReference.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/HitReference.java index 669b920923504..a11749d2cbb69 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/HitReference.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/HitReference.java @@ -14,12 +14,16 @@ public class HitReference { private final String index; private final String id; - + + public HitReference(String index, String id) { + this.index = index; + this.id = id; + } + public HitReference(SearchHit hit) { - this.index = hit.getIndex(); - this.id = hit.getId(); + this(hit.getIndex(), hit.getId()); } - + public String index() { return index; } @@ -38,11 +42,11 @@ public boolean equals(Object obj) { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + HitReference other = (HitReference) obj; return Objects.equals(index, other.index) && Objects.equals(id, other.id); diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/SequenceKey.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/SequenceKey.java index 3f9e82a3502c1..95009d5152c5e 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/SequenceKey.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/SequenceKey.java @@ -21,7 +21,7 @@ public class SequenceKey { private final Object[] keys; private final int hashCode; - public SequenceKey(Object... keys) { + SequenceKey(Object... keys) { this.keys = keys; this.hashCode = Objects.hash(keys); } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java index 29224731b0eff..48b8ccb7a7876 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java @@ -26,7 +26,9 @@ import org.elasticsearch.xpack.ql.util.ActionListeners; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.eql.execution.search.RuntimeUtils.searchHits; @@ -47,8 +49,23 @@ */ public class TumblingWindow implements Executable { + private static final int CACHE_MAX_SIZE = 63; + private final Logger log = LogManager.getLogger(TumblingWindow.class); + /** + * Simple cache for removing duplicate strings (such as index name or common keys). + * Designed to be low-effort and thus optimistic in nature. + * Thus it has a small, upper limit so that it doesn't require any cleaning up. + */ + // start with the default size and allow growth until the max size + private final Map stringCache = new LinkedHashMap<>(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return this.size() >= CACHE_MAX_SIZE; + } + }; + private final QueryClient client; private final List> criteria; private final Criterion until; @@ -522,6 +539,28 @@ private TimeValue timeTook() { return new TimeValue(System.currentTimeMillis() - startTime); } + private String cache(String string) { + String value = stringCache.putIfAbsent(string, string); + return value == null ? string : value; + } + + private SequenceKey key(Object[] keys) { + SequenceKey key; + if (keys == null) { + key = SequenceKey.NONE; + } else { + for (int i = 0; i < keys.length; i++) { + Object o = keys[i]; + if (o instanceof String) { + keys[i] = cache((String) o); + } + } + key = new SequenceKey(keys); + } + + return key; + } + private static Ordinal headOrdinal(List hits, Criterion criterion) { return criterion.ordinal(hits.get(0)); } @@ -565,9 +604,9 @@ public boolean hasNext() { @Override public Tuple next() { SearchHit hit = delegate.next(); - SequenceKey k = criterion.key(hit); + SequenceKey k = key(criterion.key(hit)); Ordinal o = criterion.ordinal(hit); - return new Tuple<>(new KeyAndOrdinal(k, o), new HitReference(hit)); + return new Tuple<>(new KeyAndOrdinal(k, o), new HitReference(cache(hit.getIndex()), hit.getId())); } }; }; From 1d0baacef81cdd9c349c336e60e15d4c3f99ff65 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Mon, 14 Dec 2020 09:50:56 +0200 Subject: [PATCH 2/2] Address feedback --- .../xpack/eql/execution/assembler/Criterion.java | 9 ++++++--- .../xpack/eql/execution/sequence/TumblingWindow.java | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java index 4fc1324bdf796..82ce19abc68e7 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/Criterion.java @@ -23,6 +23,7 @@ public class Criterion { private final HitExtractor tiebreaker; private final boolean descending; + private final int keySize; public Criterion(int stage, Q queryRequest, @@ -37,6 +38,8 @@ public Criterion(int stage, this.tiebreaker = tiebreaker; this.descending = descending; + + this.keySize = keys.size(); } public int stage() { @@ -53,9 +56,9 @@ public Q queryRequest() { public Object[] key(SearchHit hit) { Object[] key = null; - if (keys.isEmpty() == false) { - Object[] docKeys = new Object[keys.size()]; - for (int i = 0; i < docKeys.length; i++) { + if (keySize > 0) { + Object[] docKeys = new Object[keySize]; + for (int i = 0; i < keySize; i++) { docKeys[i] = keys.get(i).extract(hit); } key = docKeys; diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java index 48b8ccb7a7876..c7281368f9b30 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/sequence/TumblingWindow.java @@ -49,13 +49,13 @@ */ public class TumblingWindow implements Executable { - private static final int CACHE_MAX_SIZE = 63; + private static final int CACHE_MAX_SIZE = 64; private final Logger log = LogManager.getLogger(TumblingWindow.class); /** * Simple cache for removing duplicate strings (such as index name or common keys). - * Designed to be low-effort and thus optimistic in nature. + * Designed to be low-effort, non-concurrent (not needed) and thus optimistic in nature. * Thus it has a small, upper limit so that it doesn't require any cleaning up. */ // start with the default size and allow growth until the max size