Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Feb 14, 2025

When you load stored fields from lucene you have get all of your ducks in a row first or it'll be really slow. You need to get a list of all of the fields you want so you can ignore the ones that you don't. We do this with the StoredFieldsSpec which is immutable and has a merge method. It's quite common to merge a few dozen of these specs together to prepare for the fetch phase or for a new segment when loading fields from ESQL.

When I was loading thousands of fields in ESQL I noticed that the merge was slowing things down marginally. This skips a big chunk of the merge in the common case that we don't have to load any named stored fields.

When you load stored fields from lucene you have get all of your ducks
in a row first or it'll be really slow. You need to get a list of all of
the fields you want so you can ignore the ones that you don't. We do
this with the `StoredFieldsSpec` which is immutable and has a `merge`
method. It's quite common to merge a few dozen of these specs together
to prepare for the fetch phase or for a new segment when loading fields
from ESQL.

When I was loading thousands of fields in ESQL I noticed that the merge
was slowing things down marginally. This skips a big chunk of the merge
in the common case that we don't have to load any named stored fields.
@nik9000 nik9000 added >non-issue :Search Foundations/Search Catch all for Search Foundations v9.1.0 labels Feb 14, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Feb 14, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

would you mind adding a comment to explain that new conditional ? Do we need tests to make sure this optimizations sticks around?

@nik9000
Copy link
Member Author

nik9000 commented Mar 3, 2025

would you mind adding a comment to explain that new conditional ? Do we need tests to make sure this optimizations sticks around?

Yeah. I'll do both.

@nik9000 nik9000 added auto-backport Automatically create backport pull requests when merged v9.0.0 labels Mar 3, 2025
@nik9000 nik9000 merged commit 66a8123 into elastic:main Mar 3, 2025
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0

nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Mar 3, 2025
When you load stored fields from lucene you have get all of your ducks
in a row first or it'll be really slow. You need to get a list of all of
the fields you want so you can ignore the ones that you don't. We do
this with the `StoredFieldsSpec` which is immutable and has a `merge`
method. It's quite common to merge a few dozen of these specs together
to prepare for the fetch phase or for a new segment when loading fields
from ESQL.

When I was loading thousands of fields in ESQL I noticed that the merge
was slowing things down marginally. This skips a big chunk of the merge
in the common case that we don't have to load any named stored fields.
elasticsearchmachine pushed a commit that referenced this pull request Mar 3, 2025
When you load stored fields from lucene you have get all of your ducks
in a row first or it'll be really slow. You need to get a list of all of
the fields you want so you can ignore the ones that you don't. We do
this with the `StoredFieldsSpec` which is immutable and has a `merge`
method. It's quite common to merge a few dozen of these specs together
to prepare for the fetch phase or for a new segment when loading fields
from ESQL.

When I was loading thousands of fields in ESQL I noticed that the merge
was slowing things down marginally. This skips a big chunk of the merge
in the common case that we don't have to load any named stored fields.
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
When you load stored fields from lucene you have get all of your ducks
in a row first or it'll be really slow. You need to get a list of all of
the fields you want so you can ignore the ones that you don't. We do
this with the `StoredFieldsSpec` which is immutable and has a `merge`
method. It's quite common to merge a few dozen of these specs together
to prepare for the fetch phase or for a new segment when loading fields
from ESQL.

When I was loading thousands of fields in ESQL I noticed that the merge
was slowing things down marginally. This skips a big chunk of the merge
in the common case that we don't have to load any named stored fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >non-issue :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v9.0.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants