Skip to content

Migrates thread local free slot storage from std::list to std::vector.#44859

Merged
ggreenway merged 2 commits intoenvoyproxy:mainfrom
birenroy:no-std-list-in-thread-local
May 5, 2026
Merged

Migrates thread local free slot storage from std::list to std::vector.#44859
ggreenway merged 2 commits intoenvoyproxy:mainfrom
birenroy:no-std-list-in-thread-local

Conversation

@birenroy
Copy link
Copy Markdown
Contributor

@birenroy birenroy commented May 5, 2026

std::list is not a great choice for a performance critical utility, as it requires a heap operation on every mutation.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:

…hash_set.

Signed-off-by: Biren Roy <birenroy@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #44859 was opened by birenroy.

see: more, trace.

@birenroy birenroy marked this pull request as ready for review May 5, 2026 13:27
@birenroy
Copy link
Copy Markdown
Contributor Author

birenroy commented May 5, 2026

/assign @adisuissa

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks!
I believe that an std::vector is probably the most optimized data-structure that can be used as a stack to tackle the free_slot_indexes_ accounting (at least from a previous life experience of using different data-structures). WDYT?

Signed-off-by: Biren Roy <birenroy@google.com>
@birenroy birenroy changed the title Migrates thread local free slot storage from std::list to absl::flat_hash_set. Migrates thread local free slot storage from std::list to std::vector. May 5, 2026
@birenroy
Copy link
Copy Markdown
Contributor Author

birenroy commented May 5, 2026

Thanks! I believe that an std::vector is probably the most optimized data-structure that can be used as a stack to tackle the free_slot_indexes_ accounting (at least from a previous life experience of using different data-structures). WDYT?

Sure. The pseudo-random hash set iteration order seemed like a reasonable compromise between the FIFO of the previous version and the LIFO of a vector solution, but it probably doesn't matter.

@adisuissa
Copy link
Copy Markdown
Contributor

Sure. The pseudo-random hash set iteration order seemed like a reasonable compromise between the FIFO of the previous version and the LIFO of a vector solution, but it probably doesn't matter.

I'm not sure why FIFO was used in the first place, but I don't see a reason why it is required. I've also reviewed #7979 but didn't get any reason why other data types shouldn't be used. LIFO makes more sense IMHO.

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa 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!

@adisuissa
Copy link
Copy Markdown
Contributor

/assign-from @envoyproxy/senior-maintainers

@repokitteh-read-only
Copy link
Copy Markdown

@envoyproxy/senior-maintainers assignee is @wbpcode

🐱

Caused by: a #44859 (comment) was created by @adisuissa.

see: more, trace.

@ggreenway ggreenway enabled auto-merge (squash) May 5, 2026 16:09
@ggreenway
Copy link
Copy Markdown
Member

/retest

@ggreenway ggreenway merged commit ec1c5f3 into envoyproxy:main May 5, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants