Skip to content

Speed up block serialization #124394

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
Merged

Speed up block serialization #124394

merged 4 commits into from
Mar 10, 2025

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Mar 8, 2025

Currently, we use NamedWriteable for serializing blocks. While convenient, it incurs a noticeable performance penalty when pages contain thousands of blocks. Since block types are small and already centered in ElementType, we can safely switch from NamedWriteable to typed code. For example, the NamedWriteable alone of a small page with 10K fields would be 180KB, whereas the new method reduces it to 10KB. Below are the serialization improvements with FROM idx | LIMIT 10000 where the target index has 10K fields:

  • write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
  • read_exchange_response executed 173 times took: 49.4ms -> 25.8ms

  • I might open another PR to avoid serializing positionCount as we should already have it from the page.
  • We might need to do the same thing for plan serialization.

@dnhatn dnhatn force-pushed the serialize-block-code branch 11 times, most recently from 8fa9212 to 53f0fb8 Compare March 8, 2025 16:41
@dnhatn dnhatn force-pushed the serialize-block-code branch from 53f0fb8 to 502d522 Compare March 8, 2025 18:15
@dnhatn dnhatn changed the title Serialize block type code Avoid NamedWritable in block serialization Mar 8, 2025
@dnhatn dnhatn added v8.19.0 :Analytics/ES|QL AKA ESQL >enhancement auto-backport Automatically create backport pull requests when merged labels Mar 8, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@dnhatn dnhatn marked this pull request as ready for review March 8, 2025 19:39
@dnhatn dnhatn requested a review from nik9000 March 8, 2025 19:39
@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)

@dnhatn dnhatn requested review from luigidellaquila and costin March 8, 2025 19:39
@dnhatn dnhatn changed the title Avoid NamedWritable in block serialization Speed up block serialization Mar 9, 2025
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines +23 to +48
BOOLEAN(0, "Boolean", BlockFactory::newBooleanBlockBuilder, BooleanBlock::readFrom),
INT(1, "Int", BlockFactory::newIntBlockBuilder, IntBlock::readFrom),
LONG(2, "Long", BlockFactory::newLongBlockBuilder, LongBlock::readFrom),
FLOAT(3, "Float", BlockFactory::newFloatBlockBuilder, FloatBlock::readFrom),
DOUBLE(4, "Double", BlockFactory::newDoubleBlockBuilder, DoubleBlock::readFrom),
/**
* Blocks containing only null values.
*/
NULL("Null", (blockFactory, estimatedSize) -> new ConstantNullBlock.Builder(blockFactory)),
NULL(5, "Null", (blockFactory, estimatedSize) -> new ConstantNullBlock.Builder(blockFactory), BlockStreamInput::readConstantNullBlock),

BYTES_REF("BytesRef", BlockFactory::newBytesRefBlockBuilder),
BYTES_REF(6, "BytesRef", BlockFactory::newBytesRefBlockBuilder, BytesRefBlock::readFrom),

/**
* Blocks that reference individual lucene documents.
*/
DOC("Doc", DocBlock::newBlockBuilder),
DOC(7, "Doc", DocBlock::newBlockBuilder, in -> { throw new UnsupportedOperationException("can't read doc blocks"); }),

/**
* Composite blocks which contain array of sub-blocks.
*/
COMPOSITE("Composite", BlockFactory::newAggregateMetricDoubleBlockBuilder),
COMPOSITE(8, "Composite", BlockFactory::newAggregateMetricDoubleBlockBuilder, CompositeBlock::readFrom),

/**
* Intermediate blocks which don't support retrieving elements.
*/
UNKNOWN("Unknown", (blockFactory, estimatedSize) -> { throw new UnsupportedOperationException("can't build null blocks"); });
UNKNOWN(9, "Unknown", (blockFactory, estimatedSize) -> { throw new UnsupportedOperationException("can't build null blocks"); }, in -> {
Copy link
Member

Choose a reason for hiding this comment

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

Minute point: for future extensibility, maybe space out (multiple of two) the writeable code instead of using consecutive numbers: e.g.:
0 - null
1 - unknown
2-3 - unused
4-15: java primitives (including those not supported yet such as byte)
16-32: rest of the objects (doc, composite, etc..)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can add new element types with the next ids.

Copy link
Contributor

@luigidellaquila luigidellaquila left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Nhat!

@dnhatn
Copy link
Member Author

dnhatn commented Mar 10, 2025

Thanks everyone!

@dnhatn dnhatn merged commit 79a1626 into elastic:main Mar 10, 2025
17 checks passed
@dnhatn dnhatn deleted the serialize-block-code branch March 10, 2025 18:54
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
Currently, we use NamedWriteable for serializing blocks. While 
convenient, it incurs a noticeable performance penalty when pages 
contain thousands of blocks. Since block types are small and already
centered in ElementType, we can safely switch from NamedWriteable to
typed code. For example, the NamedWriteable alone of a small page with
10K fields would be 180KB, whereas the new method reduces it to 10KB.
Below are the serialization improvements with FROM idx | LIMIT 10000
where the target index has 10K fields:

- write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
- read_exchange_response executed 173 times took: 49.4ms -> 25.8ms
albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025
Currently, we use NamedWriteable for serializing blocks. While 
convenient, it incurs a noticeable performance penalty when pages 
contain thousands of blocks. Since block types are small and already
centered in ElementType, we can safely switch from NamedWriteable to
typed code. For example, the NamedWriteable alone of a small page with
10K fields would be 180KB, whereas the new method reduces it to 10KB.
Below are the serialization improvements with FROM idx | LIMIT 10000
where the target index has 10K fields:

- write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
- read_exchange_response executed 173 times took: 49.4ms -> 25.8ms
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025
Currently, we use NamedWriteable for serializing blocks. While 
convenient, it incurs a noticeable performance penalty when pages 
contain thousands of blocks. Since block types are small and already
centered in ElementType, we can safely switch from NamedWriteable to
typed code. For example, the NamedWriteable alone of a small page with
10K fields would be 180KB, whereas the new method reduces it to 10KB.
Below are the serialization improvements with FROM idx | LIMIT 10000
where the target index has 10K fields:

- write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
- read_exchange_response executed 173 times took: 49.4ms -> 25.8ms
@dnhatn
Copy link
Member Author

dnhatn commented Mar 13, 2025

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Mar 13, 2025
Currently, we use NamedWriteable for serializing blocks. While
convenient, it incurs a noticeable performance penalty when pages
contain thousands of blocks. Since block types are small and already
centered in ElementType, we can safely switch from NamedWriteable to
typed code. For example, the NamedWriteable alone of a small page with
10K fields would be 180KB, whereas the new method reduces it to 10KB.
Below are the serialization improvements with FROM idx | LIMIT 10000
where the target index has 10K fields:

- write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
- read_exchange_response executed 173 times took: 49.4ms -> 25.8ms

(cherry picked from commit 79a1626)
elasticsearchmachine pushed a commit that referenced this pull request Mar 14, 2025
Currently, we use NamedWriteable for serializing blocks. While
convenient, it incurs a noticeable performance penalty when pages
contain thousands of blocks. Since block types are small and already
centered in ElementType, we can safely switch from NamedWriteable to
typed code. For example, the NamedWriteable alone of a small page with
10K fields would be 180KB, whereas the new method reduces it to 10KB.
Below are the serialization improvements with FROM idx | LIMIT 10000
where the target index has 10K fields:

- write_exchange_response executed 173 times took: 73.2ms -> 26.7ms
- read_exchange_response executed 173 times took: 49.4ms -> 25.8ms

(cherry picked from commit 79a1626)
@elastic elastic deleted a comment from elasticsearchmachine Mar 20, 2025
dnhatn added a commit that referenced this pull request Mar 20, 2025
Adjust wire version after backporting to 8.x.

Relates #124394
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Mar 21, 2025
Adjust wire version after backporting to 8.x.

Relates elastic#124394
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
Adjust wire version after backporting to 8.x.

Relates elastic#124394
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 >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants