-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
Conversation
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
The update looks good to me. Are there any other classes where a similar change could be applied? I noticed, for example, that |
I looked around and I didn't found any, I actually missed |
There was a problem hiding this 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
This commit trim to size those lists to prevent wasteful heap usage.
This commit trim to size those lists to prevent wasteful heap usage.
This commit trim to size those lists to prevent wasteful heap usage.
This commit trim to size those lists to prevent wasteful heap usage.
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.