Conversation
Signed-off-by: Matt Klein <[email protected]>
| // buffer on the heap and use it for all calculations. The needed size is the size of the | ||
| // address, plus '_', plus 32 bytes for the index. All of this is done to avoid string | ||
| // allocations in the fast path. | ||
| const uint64_t needed_size = std::min(128UL, address_string.size() + 1 + 32); |
There was a problem hiding this comment.
Is there some constant we can use for max 64-bit representation? In the StringUtil::itoa comment it says this is 21 (including null), here we use 32 just to be safe (I think).
There was a problem hiding this comment.
Yeah I will add a constant. I realized this code has a bug, it should be std::max, not min. I'm going to redo it also to use https://github.com/abseil/abseil-cpp/blob/master/absl/container/inlined_vector.h which would make it stack allocated in most case and easier to read. Will also add a test that would catch the min/max issue. I will update all of it tomorrow.
Signed-off-by: Matt Klein <[email protected]>
|
@htuch updated. I decided to take a simpler approach. Update comments also. |
Follow-up to envoyproxy/envoy-mobile#2277. Signed-off-by: JP Simard <[email protected]>
Follow-up to envoyproxy/envoy-mobile#2277. Signed-off-by: JP Simard <[email protected]>
2 participants
Avoid string copies during the inner loop of the ring build.
Risk Level: Low
Testing: Existing unit tests
Docs Changes: N/A
Release Notes: N/A