Skip to content

Commit

Permalink
Speed up synthetic source again
Browse files Browse the repository at this point in the history
When I added support for stored fields to synthetic _source (elastic#87480) I
accidentally caused a performance regression. Our friends working on
building the nightly charts for tsdb caught it. It looked like:
```
|   50th percentile latency | default_1k | 20.1228 | 41.289  | 21.1662  | ms | +105.18% |
|   90th percentile latency | default_1k | 26.7402 | 42.5878 | 15.8476  | ms |  +59.27% |
|   99th percentile latency | default_1k | 37.0881 | 45.586  |  8.49786 | ms |  +22.91% |
| 99.9th percentile latency | default_1k | 43.7346 | 48.222  |  4.48742 | ms |  +10.26% |
|  100th percentile latency | default_1k | 46.057  | 56.8676 | 10.8106  | ms |  +23.47% |
```

This fixes the regression and puts us in line with how we were:
```
|   50th percentile latency | default_1k | 20.1228 | 24.023  |  3.90022 | ms |  +19.38% |
|   90th percentile latency | default_1k | 26.7402 | 29.7841 |  3.04392 | ms |  +11.38% |
|   99th percentile latency | default_1k | 37.0881 | 36.8038 | -0.28428 | ms |   -0.77% |
| 99.9th percentile latency | default_1k | 43.7346 | 39.0192 | -4.71531 | ms |  -10.78% |
|  100th percentile latency | default_1k | 46.057  | 42.9181 | -3.13889 | ms |   -6.82% |
```

A 20% bump in the 50% latency isn't great, but it four microseconds per
document which is acceptable.
  • Loading branch information
nik9000 committed Aug 24, 2022
1 parent ffcf0ea commit afee04d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,21 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
loaders.add(loader);
}
}
return docId -> {
if (loaders.isEmpty()) {
return null;
}
return new ObjectDocValuesLoader(loaders);
}

private class ObjectDocValuesLoader implements DocValuesLoader {
private final List<SourceLoader.SyntheticFieldLoader.DocValuesLoader> loaders;

private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
this.loaders = loaders;
}

@Override
public boolean advanceToDoc(int docId) throws IOException {
for (SourceLoader.SyntheticFieldLoader.DocValuesLoader docValueLoader : loaders) {
boolean leafHasValue = docValueLoader.advanceToDoc(docId);
hasValue |= leafHasValue;
Expand All @@ -616,7 +630,7 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
* stored field.
*/
return hasValue;
};
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,18 @@ public Set<String> requiredStoredFields() {

@Override
public Leaf leaf(LeafReader reader, int[] docIdsInLeaf) throws IOException {
SyntheticFieldLoader.DocValuesLoader leaf = loader.docValuesLoader(reader, docIdsInLeaf);
return (fieldsVisitor, docId) -> {
return new SyntheticLeaf(loader.docValuesLoader(reader, docIdsInLeaf));
}

private class SyntheticLeaf implements Leaf {
private final SyntheticFieldLoader.DocValuesLoader docValuesLoader;

private SyntheticLeaf(SyntheticFieldLoader.DocValuesLoader docValuesLoader) {
this.docValuesLoader = docValuesLoader;
}

@Override
public BytesReference source(FieldsVisitor fieldsVisitor, int docId) throws IOException {
if (fieldsVisitor != null) {
for (Map.Entry<String, List<Object>> e : fieldsVisitor.fields().entrySet()) {
SyntheticFieldLoader.StoredFieldLoader loader = storedFieldLoaders.get(e.getKey());
Expand All @@ -118,14 +128,14 @@ public Leaf leaf(LeafReader reader, int[] docIdsInLeaf) throws IOException {
}
// TODO accept a requested xcontent type
try (XContentBuilder b = new XContentBuilder(JsonXContent.jsonXContent, new ByteArrayOutputStream())) {
if (leaf.advanceToDoc(docId)) {
if (docValuesLoader.advanceToDoc(docId)) {
loader.write(b);
} else {
b.startObject().endObject();
}
return BytesReference.bytes(b);
}
};
}
}
}

Expand Down

0 comments on commit afee04d

Please sign in to comment.