Skip to content

Conversation

@cmcfarlen
Copy link
Contributor

Internal testing shows this change increased performance on high RPS setups enough to warrant a code change. I've conditioned this change on the existing ENABLE_MALLOC_ALLOCATOR build flag so that it's opt-in.

@cmcfarlen cmcfarlen added this to the 10.2.0 milestone Dec 19, 2025
@cmcfarlen cmcfarlen self-assigned this Dec 19, 2025
Comment on lines +213 to +217
#if TS_USE_MALLOC_ALLOCATOR
MIOBuffer _receive_buffer{BUFFER_SIZE_INDEX_FOR_XMALLOC_SIZE(4096)};
#else
MIOBuffer _receive_buffer{BUFFER_SIZE_INDEX_4K};
VIO read_vio;
VIO write_vio;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Can HTTP2_HEADER_BUFFER_SIZE_INDEX be constexpr so that _receive_buffer is allocated with that value? Or do compilers not like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue here is the visibility of HTTP2_HEADER_BUFFER_SIZE_INDEX. It's not included from this file. I didn't want to change the dependencies of the files. I presume that was why it didn't previously use the buffer size index.

I couldn't find any good common header to move the definition to either.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Thanks for interacting with the idea.

@apache apache deleted a comment from cmcfarlen Dec 30, 2025
@apache apache deleted a comment from cmcfarlen Dec 30, 2025
@apache apache deleted a comment from cmcfarlen Dec 30, 2025
@cmcfarlen cmcfarlen merged commit c98e6e3 into apache:master Jan 6, 2026
15 checks passed
@cmcfarlen cmcfarlen deleted the h2-hdr-malloc branch January 6, 2026 17:36
@github-project-automation github-project-automation bot moved this to For v10.1.1 in ATS v10.1.x Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: For v10.1.1

Development

Successfully merging this pull request may close these issues.

2 participants