Skip to content

ESQL: Lazy collection copying during node transform #124424

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
Mar 10, 2025

Conversation

costin
Copy link
Member

@costin costin commented Mar 8, 2025

A set of optimization for tree traversal:

  1. perform lazy copying during children transform
  2. use long hashing to avoid object creation
  3. perform type check first before collection checking

Relates #124395

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 8, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@costin costin added v8.19.0 v8.18.1 auto-backport Automatically create backport pull requests when merged and removed v8.9.0 labels Mar 10, 2025
Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0
8.18
8.x

costin added a commit to costin/elasticsearch that referenced this pull request Mar 10, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
costin added a commit to costin/elasticsearch that referenced this pull request Mar 10, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
costin added a commit to costin/elasticsearch that referenced this pull request Mar 10, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates #124395
elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates #124395
elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates #124395
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

Heya, late to the party but the changes LGTM.

Maybe we should add a micro-benchmark or nightly track as one of the next steps, so we can make the difference in performance more visible.

@@ -52,7 +52,7 @@ final <E> T transform(Function<? super E, ? extends E> rule, Class<E> typeToken)
List<?> children = node.children();

Function<Object, Object> realRule = p -> {
if (p != children && false == children.contains(p) && (p == null || typeToken.isInstance(p))) {
if (p != children && (p == null || typeToken.isInstance(p)) && false == children.contains(p)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thought: should the children.contains(p) still show up in profiles, we could maybe refactor NodeInfo to track the non-children properties separately - avoiding this check in the first place.

albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025
* ESQL: Lazy collection copying during node transform

A set of optimization for tree traversal:
1. perform lazy copying during children transform
2. use long hashing to avoid object creation
3. perform type check first before collection checking

Relates elastic#124395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.18.1 v8.19.0 v9.0.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants