add empty() to PriorityConnPoolMap#18057
Conversation
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
davinci26
left a comment
davinci26
left a comment
There was a problem hiding this comment.
LGTM thanks, just a nit suggestion, do you want to add a comment in the function size() to say that it iterates over the map so that callers know to prefer empty()
Linter should have reported it. After I add the empty(), my vscode starts to report it. |
davinci26
left a comment
davinci26
left a comment
There was a problem hiding this comment.
If the linter already reports it, then this is a way stronger signal than a docs comment
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
davinci26
left a comment
davinci26
left a comment
There was a problem hiding this comment.
LGTM, onto a second @envoyproxy/envoy-maintainers to review and merge
| /** | ||
| * @return true if the pools are empty. | ||
| */ | ||
| size_t empty() const; |
There was a problem hiding this comment.
should this return bool instead of size_t?
There was a problem hiding this comment.
Yes, it definitely should. @lambdai can you post a followup to fix that?
5 participants
Commit Message:
Speed up a little bit since pool is very unlikely empty().
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]