acl: Avoid allocations when checking implied ops #12114
Conversation
`acl_implied_ops` was quite visible (~3%) in the callchains leading to `operator new()` in the LRC profile. It's called for every partition so this is not entirely surprising when using larger partition counts. Avoid the allocations by "inlining" the check into the function so that no vector has to be allocated. (cherry picked from commit 74ea237)
|
lgtm. well done. |
| /* | ||
| * Compute the implied operations based on the specified operation. | ||
| */ | ||
| inline std::vector<acl_operation> acl_implied_ops(acl_operation operation) { |
There was a problem hiding this comment.
i suppose you could return const-ref iterator pointers to static constexpr arrays as a std::pair<> but this is a good design. lgtm.
There was a problem hiding this comment.
lgtm.
i suppose a fewer LOC change is to return a std::pair of pointers to begin, end of static constexpr arrays. though i bet your version just inlines the whole thing into a jump table anyway, so is even more efficient. kinda sad the compiler didn't see through this relatively simple case structure by default.
I wanted to do that first but the problem is the default cause where we return a one-sized array of the input op so that makes it difficult. |
acl_implied_opswas quite visible (~3%) in the callchains leading tooperator new()in the LRC profile.It's called for every partition so this is not entirely surprising when using larger partition counts.
Avoid the allocations by "inlining" the check into the function so that no vector has to be allocated.
(cherry picked from commit 74ea237)
Fixes #12097
Backports Required
Release Notes