Skip to content

filter state: replace std::map with absl::flat_hash_map in FilterStateImpl#6705

Merged
htuch merged 1 commit intoenvoyproxy:masterfrom
kyu-c:filter-state
Apr 25, 2019
Merged

filter state: replace std::map with absl::flat_hash_map in FilterStateImpl#6705
htuch merged 1 commit intoenvoyproxy:masterfrom
kyu-c:filter-state

Conversation

@kyu-c
Copy link
Contributor

@kyu-c kyu-c commented Apr 25, 2019

Signed-off-by: Kyu Chang kyuc@google.com

Description: Change the type of data_storage_ from std::map to absl::flat_hash_map which is unordered map and supports heterogeneous lookup natively. Cleanup absl::string_view to std::string conversion in the cc file.
Risk Level: Low
Testing: unit test
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Kyu Chang <kyuc@google.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks, this is much cleaner. Can you confirm with @AndresGuedez (Envoy google3 import on-call this week) that the string changes are safe?
/wait-any

@AndresGuedez
Copy link
Contributor

Thanks, this is much cleaner. Can you confirm with @AndresGuedez (Envoy google3 import on-call this week) that the string changes are safe?

This is safe; the problems I ran into were due to a missing constructor in std::pair<> which is not applicable to these changes.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@htuch htuch merged commit ac32db3 into envoyproxy:master Apr 25, 2019
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.

3 participants