Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up synthetic source #87882

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1078,21 +1078,27 @@ public BytesSyntheticFieldLoader(String name, String simpleName) {
@Override
public Leaf leaf(LeafReader reader) throws IOException {
SortedSetDocValues leaf = DocValues.getSortedSet(reader, name);
if (leaf.getValueCount() == 0) {
return SourceLoader.SyntheticFieldLoader.NOTHING.leaf(reader);
}
return new SourceLoader.SyntheticFieldLoader.Leaf() {
private boolean hasValue;

@Override
public void advanceToDoc(int docId) throws IOException {
hasValue = leaf.advanceExact(docId);
public boolean empty() {
return false;
}

@Override
public boolean hasValue() {
return hasValue;
public boolean advanceToDoc(int docId) throws IOException {
return hasValue = leaf.advanceExact(docId);
}

@Override
public void load(XContentBuilder b) throws IOException {
public void write(XContentBuilder b) throws IOException {
if (false == hasValue) {
return;
}
long first = leaf.nextOrd();
long next = leaf.nextOrd();
if (next == SortedSetDocValues.NO_MORE_ORDS) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.apache.lucene.index.DocValues;
import org.apache.lucene.index.LeafReader;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.sandbox.document.HalfFloatPoint;
import org.apache.lucene.sandbox.search.IndexSortSortedNumericDocValuesRangeQuery;
Expand Down Expand Up @@ -1631,22 +1632,28 @@ protected NumericSyntheticFieldLoader(String name, String simpleName) {

@Override
public Leaf leaf(LeafReader reader) throws IOException {
SortedNumericDocValues leaf = DocValues.getSortedNumeric(reader, name);
SortedNumericDocValues leaf = dv(reader);
if (leaf == null) {
return SourceLoader.SyntheticFieldLoader.NOTHING.leaf(reader);
}
return new SourceLoader.SyntheticFieldLoader.Leaf() {
private boolean hasValue;

@Override
public void advanceToDoc(int docId) throws IOException {
hasValue = leaf.advanceExact(docId);
public boolean empty() {
return false;
}

@Override
public boolean hasValue() {
return hasValue;
public boolean advanceToDoc(int docId) throws IOException {
return hasValue = leaf.advanceExact(docId);
}

@Override
public void load(XContentBuilder b) throws IOException {
public void write(XContentBuilder b) throws IOException {
if (false == hasValue) {
return;
}
if (leaf.docValueCount() == 1) {
b.field(simpleName);
loadNextValue(b, leaf.nextValue());
Expand All @@ -1662,5 +1669,23 @@ public void load(XContentBuilder b) throws IOException {
}

protected abstract void loadNextValue(XContentBuilder b, long value) throws IOException;

/**
* Returns a {@link SortedNumericDocValues} or null if it doesn't have any doc values.
* See {@link DocValues#getSortedNumeric} which is *nearly* the same, but it returns
* an "empty" implementation if there aren't any doc values. We need to be able to
* tell if there aren't any and return our empty leaf source loader.
*/
private SortedNumericDocValues dv(LeafReader reader) throws IOException {
SortedNumericDocValues dv = reader.getSortedNumericDocValues(name);
if (dv != null) {
return dv;
}
NumericDocValues single = reader.getNumericDocValues(name);
if (single != null) {
return DocValues.singleton(single);
}
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -554,53 +554,51 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep

@Override
public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
List<SourceLoader.SyntheticFieldLoader> fields = new ArrayList<>();
mappers.values().stream().sorted(Comparator.comparing(Mapper::name)).forEach(sub -> {
SourceLoader.SyntheticFieldLoader subLoader = sub.syntheticFieldLoader();
if (subLoader != null) {
fields.add(subLoader);
}
});
List<SourceLoader.SyntheticFieldLoader> fields = mappers.values()
.stream()
.sorted(Comparator.comparing(Mapper::name))
.map(Mapper::syntheticFieldLoader)
.filter(l -> l != null)
.toList();
return new SourceLoader.SyntheticFieldLoader() {
@Override
public Leaf leaf(LeafReader reader) throws IOException {
List<SourceLoader.SyntheticFieldLoader.Leaf> leaves = new ArrayList<>();
List<SourceLoader.SyntheticFieldLoader.Leaf> l = new ArrayList<>();
for (SourceLoader.SyntheticFieldLoader field : fields) {
leaves.add(field.leaf(reader));
Leaf leaf = field.leaf(reader);
if (false == leaf.empty()) {
l.add(leaf);
}
}
SourceLoader.SyntheticFieldLoader.Leaf[] leaves = l.toArray(SourceLoader.SyntheticFieldLoader.Leaf[]::new);
return new SourceLoader.SyntheticFieldLoader.Leaf() {
private boolean hasValue;

@Override
public void advanceToDoc(int docId) throws IOException {
for (SourceLoader.SyntheticFieldLoader.Leaf leaf : leaves) {
leaf.advanceToDoc(docId);
}
public boolean empty() {
return leaves.length == 0;
}

@Override
public boolean hasValue() {
public boolean advanceToDoc(int docId) throws IOException {
hasValue = false;
for (SourceLoader.SyntheticFieldLoader.Leaf leaf : leaves) {
if (leaf.hasValue()) {
return true;
}
boolean leafHasValue = leaf.advanceToDoc(docId);
hasValue |= leafHasValue;
}
return false;
return hasValue;
}

@Override
public void load(XContentBuilder b) throws IOException {
boolean started = false;
for (SourceLoader.SyntheticFieldLoader.Leaf leaf : leaves) {
if (leaf.hasValue()) {
if (false == started) {
started = true;
startSyntheticField(b);
}
leaf.load(b);
}
public void write(XContentBuilder b) throws IOException {
if (hasValue == false) {
return;
}
if (started) {
b.endObject();
startSyntheticField(b);
for (SourceLoader.SyntheticFieldLoader.Leaf leaf : leaves) {
leaf.write(b);
}
b.endObject();
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,24 @@ public boolean reordersFieldValues() {
@Override
public Leaf leaf(LeafReader reader) throws IOException {
SyntheticFieldLoader.Leaf leaf = loader.leaf(reader);
if (leaf.empty()) {
return new Leaf() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this has no state I wonder if it's worth having it as a final instance on Leaf?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Override
public BytesReference source(FieldsVisitor fieldsVisitor, int docId) throws IOException {
// TODO accept a requested xcontent type
try (XContentBuilder b = new XContentBuilder(JsonXContent.jsonXContent, new ByteArrayOutputStream())) {
return BytesReference.bytes(b.startObject().endObject());
}
}
};
}
return new Leaf() {
@Override
public BytesReference source(FieldsVisitor fieldsVisitor, int docId) throws IOException {
// TODO accept a requested xcontent type
try (XContentBuilder b = new XContentBuilder(JsonXContent.jsonXContent, new ByteArrayOutputStream())) {
leaf.advanceToDoc(docId);
if (leaf.hasValue()) {
leaf.load(b);
if (leaf.advanceToDoc(docId)) {
leaf.write(b);
} else {
b.startObject().endObject();
}
Expand All @@ -104,20 +114,19 @@ public BytesReference source(FieldsVisitor fieldsVisitor, int docId) throws IOEx
* Load a field for {@link Synthetic}.
*/
interface SyntheticFieldLoader {
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still worth having some javadoc on this I think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what I did there.

* Load no values.
*/
SyntheticFieldLoader NOTHING = r -> new Leaf() {
@Override
public void advanceToDoc(int docId) throws IOException {}
public boolean empty() {
return true;
}

@Override
public boolean hasValue() {
public boolean advanceToDoc(int docId) throws IOException {
return false;
}

@Override
public void load(XContentBuilder b) throws IOException {}
public void write(XContentBuilder b) throws IOException {}
};

/**
Expand All @@ -130,19 +139,19 @@ public void load(XContentBuilder b) throws IOException {}
*/
interface Leaf {
/**
* Position the loader at a document.
* Is this entirely empty?
*/
void advanceToDoc(int docId) throws IOException;
boolean empty();

/**
* Is there a value for this field in this document?
* Position the loader at a document.
*/
boolean hasValue();
boolean advanceToDoc(int docId) throws IOException;

/**
* Load values for this document.
* Write values for this document.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

*/
void load(XContentBuilder b) throws IOException;
void write(XContentBuilder b) throws IOException;
}
}

Expand Down