Skip to content

Trim to size lists created in source fetchers #130521

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

Merged
merged 4 commits into from
Jul 4, 2025

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Jul 3, 2025

Looking at a heap dump, in most of the cases we will have just one value, still those lists allocate 10 slots which can be pretty wasteful. This commit trim to size those lists to prevent wasteful heap usage.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Jul 3, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @iverase, I've created a changelog YAML for you.

@@ -56,7 +56,7 @@ public ArraySourceValueFetcher(Set<String> sourcePaths, Object nullValue) {

@Override
public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValues) {
List<Object> values = new ArrayList<>();
ArrayList<Object> values = new ArrayList<>();
for (String path : sourcePaths) {
Object sourceValue = source.extractValue(path, nullValue);
if (sourceValue == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't directly related to your changes. Still, it appears that the code exits early on the first null value, potentially skipping other paths that may contain valid data. Do you happen to know if that behavior is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has always been like that so I would say it is intentional.

@drempapis
Copy link
Contributor

The update looks good to me. Are there any other classes where a similar change could be applied? I noticed, for example, that NestedValueFetcher might be a candidate for the same update.

@iverase
Copy link
Contributor Author

iverase commented Jul 3, 2025

Are there any other classes where a similar change could be applied? I noticed, for example, that NestedValueFetcher might be a candidate for the same update.

I looked around and I didn't found any, I actually missed NestedValueFetcher, I changed now.

Copy link
Contributor

@drempapis drempapis left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @iverase

@iverase iverase added the auto-backport Automatically create backport pull requests when merged label Jul 3, 2025
@iverase iverase merged commit bc02964 into elastic:main Jul 4, 2025
32 checks passed
@iverase iverase deleted the trimSourceFetcher branch July 4, 2025 06:21
iverase added a commit to iverase/elasticsearch that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0
9.1
8.18
8.19

iverase added a commit to iverase/elasticsearch that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
iverase added a commit to iverase/elasticsearch that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
iverase added a commit to iverase/elasticsearch that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2025
This commit trim to size those lists to prevent wasteful heap usage.
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 >bug :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.18.4 v8.19.1 v9.0.0 v9.1.0 v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants