Skip to content

Stats speed test#4867

Merged
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
jmarantz:stats-speed-test
Oct 28, 2018
Merged

Stats speed test#4867
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
jmarantz:stats-speed-test

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz commented Oct 26, 2018

Description: adds a speed-test measuring the single-threaded performance of the stats store with and without tls. of course every machine is different, but on mine a typical run gives results similar to this:

bazel-bin/test/common/stats/thread_local_store_speed_test \
 --benchmark_report_aggregates_only=true --benchmark_repetitions=100
--------------------------------------------------------------
Benchmark                       Time           CPU Iterations
--------------------------------------------------------------
BM_StatsNoTls_mean       16249144 ns   16248983 ns         37
BM_StatsNoTls_median     16176011 ns   16175443 ns         37
BM_StatsNoTls_stddev       484905 ns     484909 ns         37
BM_StatsWithTls_mean     20958468 ns   20958244 ns         30
BM_StatsWithTls_median   20899829 ns   20899217 ns         30
BM_StatsWithTls_stddev     576702 ns     576784 ns         30

with variances in the single-digit percent range. This should help quantify the perf impact of potential changes to the stats system, to justify #4711 and #4715

Risk Level: low
Testing: only the new test
Docs Changes:
Release Notes:

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

private:
Stats::StatsOptionsImpl options_;
Event::SimulatedTimeSystem time_system_;
Stats::HeapStatDataAllocator heap_alloc_;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth it to have a variant of this test that use shared memory? I'm guessing for most of the things we care about (cached fast path) it doesn't really matter, but possibly worth putting in a TODO like you did for the MT thread contention case below?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good idea; done.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 merged commit 774426b into envoyproxy:master Oct 28, 2018
@jmarantz jmarantz deleted the stats-speed-test branch October 29, 2018 01:12
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