Skip to content

admin: re-enable fast admin stats#20887

Merged
jmarantz merged 3 commits intoenvoyproxy:mainfrom
jmarantz:re-enable-fast-admin-stats
Apr 20, 2022
Merged

admin: re-enable fast admin stats#20887
jmarantz merged 3 commits intoenvoyproxy:mainfrom
jmarantz:re-enable-fast-admin-stats

Conversation

@jmarantz
Copy link
Contributor

@jmarantz jmarantz commented Apr 19, 2022

Commit Message: Rolls back the rollback PR #20835 , re-enabling fast admin stats, now that #20855 has landed.
Additional Description:
This brings the performance back to this state:

------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_AllCountersText             467 ms          466 ms            2
BM_UsedCountersText           37.0 ms         37.0 ms           19
BM_FilteredCountersText       1793 ms         1792 ms            1
BM_AllCountersJson             504 ms          504 ms            1
BM_UsedCountersJson           37.2 ms         37.2 ms           19
BM_FilteredCountersJson       1839 ms         1839 ms            1

So: around half a second of CPU burst for 1M json & text stats, rather than 1.8 seconds for text and 4.7 seconds for json. We still have a std::regex bottleneck when a filter is specified.
Risk Level: medium -- this re-enables calling of code that previously had had races, though #20855 repro'd and fixes them.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #20887 was opened by jmarantz.

see: more, trace.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz marked this pull request as ready for review April 19, 2022 20:12
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz merged commit c90728a into envoyproxy:main Apr 20, 2022
@jmarantz jmarantz deleted the re-enable-fast-admin-stats branch April 20, 2022 20:51
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Commit Message: Rolls back the rollback PR envoyproxy#20835 , re-enabling fast admin stats, now that envoyproxy#20855 has landed.
Additional Description:
This brings the performance back to this state:
```
------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_AllCountersText             467 ms          466 ms            2
BM_UsedCountersText           37.0 ms         37.0 ms           19
BM_FilteredCountersText       1793 ms         1792 ms            1
BM_AllCountersJson             504 ms          504 ms            1
BM_UsedCountersJson           37.2 ms         37.2 ms           19
BM_FilteredCountersJson       1839 ms         1839 ms            1
```
So: around half a second of CPU burst for 1M json & text stats, rather than 1.8 seconds for text and 4.7 seconds for json. We still have a std::regex bottleneck when a filter is specified.
Risk Level: medium -- this re-enables calling of code that previously had had races, though envoyproxy#20855 repro'd and fixes them.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
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.

2 participants