From 76e0c15a064bdf009eb4971328ec2821c6f1bb0a Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 3 Dec 2019 16:59:52 +0100 Subject: [PATCH 1/7] Add a reusable implementation of HistogramValue so we do not create an object per document --- .../mapper/HistogramFieldMapper.java | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 6ef920bd33fa3..02dc15fd67220 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -202,7 +202,9 @@ public AtomicHistogramFieldData load(LeafReaderContext context) { public HistogramValues getHistogramValues() throws IOException { try { final BinaryDocValues values = DocValues.getBinary(context.reader(), fieldName); + final InternalHistogramValue value = new InternalHistogramValue(); return new HistogramValues() { + @Override public boolean advanceExact(int doc) throws IOException { return values.advanceExact(doc); @@ -211,7 +213,8 @@ public boolean advanceExact(int doc) throws IOException { @Override public HistogramValue histogram() throws IOException { try { - return getHistogramValue(values.binaryValue()); + value.reset(values.binaryValue()); + return value; } catch (IOException e) { throw new IOException("Cannot load doc value", e); } @@ -220,7 +223,6 @@ public HistogramValue histogram() throws IOException { } catch (IOException e) { throw new IOException("Cannot load doc values", e); } - } @Override @@ -259,44 +261,6 @@ public SortField sortField(Object missingValue, MultiValueMode sortMode, } }; } - - private HistogramValue getHistogramValue(final BytesRef bytesRef) throws IOException { - final ByteBufferStreamInput streamInput = new ByteBufferStreamInput( - ByteBuffer.wrap(bytesRef.bytes, bytesRef.offset, bytesRef.length)); - return new HistogramValue() { - double value; - int count; - boolean isExhausted; - - @Override - public boolean next() throws IOException { - if (streamInput.available() > 0) { - count = streamInput.readVInt(); - value = streamInput.readDouble(); - return true; - } - isExhausted = true; - return false; - } - - @Override - public double value() { - if (isExhausted) { - throw new IllegalArgumentException("histogram already exhausted"); - } - return value; - } - - @Override - public int count() { - if (isExhausted) { - throw new IllegalArgumentException("histogram already exhausted"); - } - return count; - } - }; - } - }; } @@ -439,4 +403,50 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, builder.field(Names.IGNORE_MALFORMED, ignoreMalformed.value()); } } + + /** re-usable {@link HistogramValue} implementation */ + private static class InternalHistogramValue extends HistogramValue { + double value; + int count; + boolean isExhausted; + ByteBufferStreamInput streamInput; + + public InternalHistogramValue() { + } + + /** reset the value for the histogram */ + public void reset(BytesRef bytesRef) { + streamInput = new ByteBufferStreamInput(ByteBuffer.wrap(bytesRef.bytes, bytesRef.offset, bytesRef.length)); + isExhausted = false; + value = 0; + count = 0; + } + + @Override + public boolean next() throws IOException { + if (streamInput.available() > 0) { + count = streamInput.readVInt(); + value = streamInput.readDouble(); + return true; + } + isExhausted = true; + return false; + } + + @Override + public double value() { + if (isExhausted) { + throw new IllegalArgumentException("histogram already exhausted"); + } + return value; + } + + @Override + public int count() { + if (isExhausted) { + throw new IllegalArgumentException("histogram already exhausted"); + } + return count; + } + } } From 990b3baeba76df988f0b42277796a796b77e8416 Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 3 Dec 2019 17:13:57 +0100 Subject: [PATCH 2/7] checkStyle --- .../xpack/analytics/mapper/HistogramFieldMapper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 02dc15fd67220..ce332c6440723 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -411,11 +411,11 @@ private static class InternalHistogramValue extends HistogramValue { boolean isExhausted; ByteBufferStreamInput streamInput; - public InternalHistogramValue() { + InternalHistogramValue() { } /** reset the value for the histogram */ - public void reset(BytesRef bytesRef) { + void reset(BytesRef bytesRef) { streamInput = new ByteBufferStreamInput(ByteBuffer.wrap(bytesRef.bytes, bytesRef.offset, bytesRef.length)); isExhausted = false; value = 0; From b43a086a16d684d83e2dfcb3d0b13af895c907d4 Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 3 Dec 2019 18:31:34 +0100 Subject: [PATCH 3/7] Deserialize using ByteArrayDataInput --- .../analytics/mapper/HistogramFieldMapper.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index ce332c6440723..563e58005748c 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -18,10 +18,12 @@ import org.apache.lucene.search.DocValuesFieldExistsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.SortField; +import org.apache.lucene.store.ByteArrayDataInput; +import org.apache.lucene.store.ByteArrayDataOutput; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.io.stream.ByteBufferStreamInput; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -368,7 +370,7 @@ public void parse(ParseContext context) throws IOException { } else if (count > 0) { // we do not add elements with count == 0 streamOutput.writeVInt(count); - streamOutput.writeDouble(values.get(i)); + streamOutput.writeLong(NumericUtils.doubleToSortableLong(values.get(i))); } } @@ -409,24 +411,25 @@ private static class InternalHistogramValue extends HistogramValue { double value; int count; boolean isExhausted; - ByteBufferStreamInput streamInput; + ByteArrayDataInput streamInput; InternalHistogramValue() { + streamInput = new ByteArrayDataInput(); } /** reset the value for the histogram */ void reset(BytesRef bytesRef) { - streamInput = new ByteBufferStreamInput(ByteBuffer.wrap(bytesRef.bytes, bytesRef.offset, bytesRef.length)); + streamInput.reset(bytesRef.bytes, bytesRef.offset, bytesRef.length); isExhausted = false; value = 0; count = 0; } @Override - public boolean next() throws IOException { - if (streamInput.available() > 0) { + public boolean next() { + if (streamInput.eof() == false) { count = streamInput.readVInt(); - value = streamInput.readDouble(); + value = NumericUtils.sortableLongToDouble(streamInput.readLong()); return true; } isExhausted = true; From dac0fb168c45fdff937ca165bac1162e97a840f6 Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 3 Dec 2019 18:34:19 +0100 Subject: [PATCH 4/7] remove unused imports --- .../xpack/analytics/mapper/HistogramFieldMapper.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 563e58005748c..2433e61b63348 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -19,7 +19,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.SortField; import org.apache.lucene.store.ByteArrayDataInput; -import org.apache.lucene.store.ByteArrayDataOutput; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; @@ -51,7 +50,6 @@ import org.elasticsearch.search.MultiValueMode; import java.io.IOException; -import java.nio.ByteBuffer; import java.util.Iterator; import java.util.List; import java.util.Map; From 876d97f943a9f362300ba065712b24c630a1cc89 Mon Sep 17 00:00:00 2001 From: iverase Date: Wed, 4 Dec 2019 09:39:16 +0100 Subject: [PATCH 5/7] Use ByteBuffersDataOutput --- .../analytics/mapper/HistogramFieldMapper.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 2433e61b63348..a17e8816dd9bd 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -19,11 +19,11 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.SortField; import org.apache.lucene.store.ByteArrayDataInput; +import org.apache.lucene.store.ByteBuffersDataOutput; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -359,7 +359,7 @@ public void parse(ParseContext context) throws IOException { "[" + COUNTS_FIELD.getPreferredName() +"] but got [" + values.size() + " != " + counts.size() +"]"); } if (fieldType().hasDocValues()) { - BytesStreamOutput streamOutput = new BytesStreamOutput(); + ByteBuffersDataOutput dataOutput = new ByteBuffersDataOutput(); for (int i = 0; i < values.size(); i++) { int count = counts.get(i); if (count < 0) { @@ -367,13 +367,12 @@ public void parse(ParseContext context) throws IOException { + name() + "], ["+ COUNTS_FIELD + "] elements must be >= 0 but got " + counts.get(i)); } else if (count > 0) { // we do not add elements with count == 0 - streamOutput.writeVInt(count); - streamOutput.writeLong(NumericUtils.doubleToSortableLong(values.get(i))); + dataOutput.writeVInt(count); + dataOutput.writeLong(NumericUtils.doubleToSortableLong(values.get(i))); } } - - Field field = new BinaryDocValuesField(simpleName(), streamOutput.bytes().toBytesRef()); - streamOutput.close(); + BytesRef docValue = new BytesRef(dataOutput.toArrayCopy(), 0, Math.toIntExact(dataOutput.size())); + Field field = new BinaryDocValuesField(simpleName(), docValue); if (context.doc().getByKey(fieldType().name()) != null) { throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't not support indexing multiple values for the same field in the same document"); From 85de8a046734dfc6b2f60bca459aa3cbdd0c6087 Mon Sep 17 00:00:00 2001 From: iverase Date: Wed, 4 Dec 2019 09:45:51 +0100 Subject: [PATCH 6/7] Use Double to bits methods instead of NumericUtils --- .../xpack/analytics/mapper/HistogramFieldMapper.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index a17e8816dd9bd..fd85ffdffe02d 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -21,7 +21,6 @@ import org.apache.lucene.store.ByteArrayDataInput; import org.apache.lucene.store.ByteBuffersDataOutput; import org.apache.lucene.util.BytesRef; -import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; @@ -368,7 +367,7 @@ public void parse(ParseContext context) throws IOException { } else if (count > 0) { // we do not add elements with count == 0 dataOutput.writeVInt(count); - dataOutput.writeLong(NumericUtils.doubleToSortableLong(values.get(i))); + dataOutput.writeLong(Double.doubleToRawLongBits(values.get(i))); } } BytesRef docValue = new BytesRef(dataOutput.toArrayCopy(), 0, Math.toIntExact(dataOutput.size())); @@ -426,7 +425,7 @@ void reset(BytesRef bytesRef) { public boolean next() { if (streamInput.eof() == false) { count = streamInput.readVInt(); - value = NumericUtils.sortableLongToDouble(streamInput.readLong()); + value = Double.longBitsToDouble(streamInput.readLong()); return true; } isExhausted = true; From 56c8ca922ea2e38c88f4b3e2a7adf61ab9231401 Mon Sep 17 00:00:00 2001 From: iverase Date: Wed, 4 Dec 2019 09:58:47 +0100 Subject: [PATCH 7/7] rename streamInput to dataInput --- .../xpack/analytics/mapper/HistogramFieldMapper.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index fd85ffdffe02d..b22f6eb0573df 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -407,15 +407,15 @@ private static class InternalHistogramValue extends HistogramValue { double value; int count; boolean isExhausted; - ByteArrayDataInput streamInput; + ByteArrayDataInput dataInput; InternalHistogramValue() { - streamInput = new ByteArrayDataInput(); + dataInput = new ByteArrayDataInput(); } /** reset the value for the histogram */ void reset(BytesRef bytesRef) { - streamInput.reset(bytesRef.bytes, bytesRef.offset, bytesRef.length); + dataInput.reset(bytesRef.bytes, bytesRef.offset, bytesRef.length); isExhausted = false; value = 0; count = 0; @@ -423,9 +423,9 @@ void reset(BytesRef bytesRef) { @Override public boolean next() { - if (streamInput.eof() == false) { - count = streamInput.readVInt(); - value = Double.longBitsToDouble(streamInput.readLong()); + if (dataInput.eof() == false) { + count = dataInput.readVInt(); + value = Double.longBitsToDouble(dataInput.readLong()); return true; } isExhausted = true;