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.

Sign up for GitHub

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

Increase flatten performance of SearchSource #150658

Merged
merged 5 commits into from
Feb 16, 2023
Merged

Increase flatten performance of SearchSource #150658

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd3f702
increase flatten performance
kertal Feb 8, 2023
8e596ad
Improve flatten performance
kertal Feb 9, 2023
39685c2
Merge branch 'main' into search-source-increase-flatten-performance
kertal Feb 9, 2023
c7e1b2f
Merge branch 'main' into search-source-increase-flatten-performance
kibanamachine Feb 15, 2023
d0ea4cc
Merge branch 'main' into search-source-increase-flatten-performance
kibanamachine Feb 16, 2023
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
54 changes: 22 additions & 32 deletions src/plugins/data/common/search/search_source/search_source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,7 @@
*/

import { setWith } from '@kbn/safer-lodash-set';
import {
difference,
isEqual,
isFunction,
isObject,
keyBy,
pick,
uniqueId,
uniqWith,
concat,
} from 'lodash';
import { difference, isEqual, isFunction, isObject, keyBy, pick, uniqueId, concat } from 'lodash';
import {
catchError,
finalize,
Expand Down Expand Up @@ -879,28 +869,28 @@ export class SearchSource {
// inject the format from the computed fields if one isn't given
const docvaluesIndex = keyBy(filteredDocvalueFields, 'field');
const bodyFields = this.getFieldsWithoutSourceFilters(index, body.fields);
body.fields = uniqWith(
bodyFields.concat(filteredDocvalueFields),
(fld1: SearchFieldValue, fld2: SearchFieldValue) => {
const field1Name = this.getFieldName(fld1);
const field2Name = this.getFieldName(fld2);
return field1Name === field2Name;

const uniqueFieldNames = new Set();
const uniqueFields = [];
for (const field of bodyFields.concat(filteredDocvalueFields)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also change to forEach here instead of for to minimize event loop blocking.

Copy link
Member Author

Choose a reason for hiding this comment

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

thx, you mean?

bodyFields.concat(filteredDocvalueFields).forEach right? wouldn't this also block the event loop?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. Well, it will block in chunks but it might also allow some browser work in between. Or maybe I am just making this up 😀

Copy link
Member Author

@kertal kertal Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think it would be the case if it was an async/await type of forEach, but here I think it's blocking the even loop until done (pls prove me wrong 😃 )

const fieldName = this.getFieldName(field);
if (metaFields.includes(fieldName) || uniqueFieldNames.has(fieldName)) {
continue;
}
)
.filter((fld: SearchFieldValue) => {
return !metaFields.includes(this.getFieldName(fld));
})
.map((fld: SearchFieldValue) => {
const fieldName = this.getFieldName(fld);
if (Object.keys(docvaluesIndex).includes(fieldName)) {
// either provide the field object from computed docvalues,
// or merge the user-provided field with the one in docvalues
return typeof fld === 'string'
? docvaluesIndex[fld]
: this.getFieldFromDocValueFieldsOrIndexPattern(docvaluesIndex, fld, index);
}
return fld;
});
uniqueFieldNames.add(fieldName);
if (Object.keys(docvaluesIndex).includes(fieldName)) {
// either provide the field object from computed docvalues,
// or merge the user-provided field with the one in docvalues
uniqueFields.push(
typeof field === 'string'
? docvaluesIndex[field]
: this.getFieldFromDocValueFieldsOrIndexPattern(docvaluesIndex, field, index)
);
} else {
uniqueFields.push(field);
}
}
body.fields = uniqueFields;
}
} else {
body.fields = filteredDocvalueFields;
Expand Down