Skip to content

Conversation

@idegtiarenko
Copy link
Contributor

This pre-sizes collections with wisible resizing cost in profile output.

@idegtiarenko idegtiarenko added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.1.0 labels Apr 7, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

} else {
inputId = ne.id();
}
Map<Integer, Layout.ChannelSet> inputChannelToOutputIds = Maps.newHashMapWithExpectedSize(projections.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

We aren't using this collection from what I see? We're just doing a .get(), but never adding things

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, why do we even spend time and resources populating it then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder, since that map is not used, does it mean we might duplicate actual data in the actual layout?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

I think this map is unused for forever now, and people found this redundant map multiple times. I was sure one of the times, we actually had merged the fix...

I'm not sure how the channel sets were intended to be used for project, but the current project operator does shallow copies of blocks, and this works well and is intended. It didn't always do this, we had to first make it work by making blocks refcounted. Maybe before that, the channel sets were supposed to help with deduplication.

@idegtiarenko idegtiarenko merged commit c2d0c59 into elastic:main Apr 8, 2025
17 checks passed
@idegtiarenko idegtiarenko deleted the presize_collections branch April 8, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants