Speedups to FakeSymbolTableTest based on microbenchmarks from #6161. #6293
Conversation
…oxy#6161. Signed-off-by: Joshua Marantz <[email protected]>
…ternals to friendds. Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
There was a problem hiding this comment.
I have a lot of very small nits and questions but overall this LGTM.
My main issue is that this PR adds a bit of temporary complexity without a manifest for which of the new methods get removed and when. If you have github issues for the future work, please add TODO(issue/<num>) or TODO(jmarantz): Remove after issue/num comments inline.
Signed-off-by: Joshua Marantz <[email protected]>
|
/retest |
|
🔨 rebuilding |
|
/retest |
|
🔨 rebuilding |
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
|
@mattklein123 I think this is ready for a look. |
Signed-off-by: Joshua Marantz <[email protected]>
|
@zuercher would you have time to look at this? Nothing too dramatic in here I think. This lacks a cross-company review so I don't want to ask Harvey or Alyssa. |
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
include/envoy/stats/symbol_table.h
Outdated
| * @param num_names The number of names. | ||
| * @param symbol_table The symbol table in which to encode the names. | ||
| */ | ||
| virtual void populateList(absl::string_view* names, int32_t num_names, StatNameList& list) PURE; |
There was a problem hiding this comment.
nit: s/int32_t/uint32_t
Where is this storage coming from? Do we need to use a pointer/length vs. using std:array/std::vector or something like that?
include/envoy/stats/symbol_table.h
Outdated
| /** | ||
| * Since SymbolTable does manual reference counting, a client of SymbolTable | ||
| * must manually call free(symbol_vec) when it is freeing the backing store | ||
| * for a StatName. This way, the symbol table will grow and shrink | ||
| * dynamically, instead of being write-only. | ||
| * dynamically, instead of being write-only |
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
|
/retest |
|
🔨 rebuilding |
Description: Pulls out a suite of improvements to FakeSymbolTable from #6161 -- that PR has grown too big to and this one of a series of logical partitions that can be broken out and reviewed. #6161 itself was intended as a less risky stepping stone to #4980 which changes the base representation fo all stat names to use a symbol table.
Risk Level: low currently -- this is not used yet, but in another PR it soon will be.
Testing: //test/common/stats/...
Docs Changes: n/a
Release Notes: n/a