Skip to content

Allow BytesRefArray to be shared#141968

Merged
dnhatn merged 3 commits intoelastic:mainfrom
dnhatn:bytes-array
Feb 6, 2026
Merged

Allow BytesRefArray to be shared#141968
dnhatn merged 3 commits intoelastic:mainfrom
dnhatn:bytes-array

Conversation

@dnhatn
Copy link
Copy Markdown
Member

@dnhatn dnhatn commented Feb 5, 2026

Currently, when building keys from BytesRefBlockHash and BytesRefLongBlockHash (used in time-series), each key must be copied. While this is acceptable for a small number of groups, the overhead becomes significant with 1M+ groups: it requires roughly twice the memory and can take seconds. This change makes BytesRefArray a RefCounted, allowing it to be shared instead of copied.

@dnhatn dnhatn force-pushed the bytes-array branch 3 times, most recently from 49302a1 to d3ca49f Compare February 6, 2026 04:57
@dnhatn dnhatn changed the title Allow sharing BytesRef from BytesRefHash Allow BytesRefArray to be shared Feb 6, 2026
@dnhatn dnhatn marked this pull request as ready for review February 6, 2026 06:38
@dnhatn dnhatn requested a review from a team as a code owner February 6, 2026 06:38
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 6, 2026
@@ -246,7 +246,8 @@ public BytesRefArray getBytesRefs() {

public BytesRefArray takeBytesRefsOwnership() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: maybe rename, e.g. sharedBytesRefs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

++ I removed it in 99ba56a

Copy link
Copy Markdown
Member

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

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

Nice, low hanging fruit.

Copy link
Copy Markdown
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

nice! 👍

@dnhatn
Copy link
Copy Markdown
Member Author

dnhatn commented Feb 6, 2026

Thanks friends!

@dnhatn dnhatn merged commit f07a037 into elastic:main Feb 6, 2026
35 checks passed
@dnhatn dnhatn deleted the bytes-array branch February 6, 2026 20:26
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.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants