Skip to content

stats: Capture all StatNames needed in the hystrix filter at construction time.#7046

Merged
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
jmarantz:hystrix-with-stat-names
May 28, 2019
Merged

stats: Capture all StatNames needed in the hystrix filter at construction time.#7046
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
jmarantz:hystrix-with-stat-names

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

Description: To avoid taking locks in the hot-path, captures all the tokens needed to compose stat-names in the Hystrix sink when it's constructed. This avoids taking locks in the hot-path.

Also adds both real-symbol-table and fake-symbol-table variants to codes_speed_test.cc. On my machine this gets:

----------------------------------------------------------------------
Benchmark                            Time             CPU   Iterations
----------------------------------------------------------------------
BM_AddResponsesFakeSymtab         8284 ns         8284 ns        79715
BM_ResponseTimingFakeSymtab        597 ns          597 ns      1164865
BM_AddResponsesRealSymtab         4389 ns         4389 ns       163165
BM_ResponseTimingRealSymtab        262 ns          262 ns      2669827

Risk Level: low
Testing: hystrix_test and the speed test
Docs Changes: n/a
Release Notes: n/a

jmarantz added 2 commits May 22, 2019 17:32
…ymbolTables in codes_speed_test.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
ahedberg
ahedberg previously approved these changes May 23, 2019
ambuc
ambuc previously approved these changes May 23, 2019
Copy link
Copy Markdown
Contributor

@ambuc ambuc left a comment

Choose a reason for hiding this comment

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

LGTM, almost no comments. This is an encouraging PR demonstrating that we can move off string-based stat names!

jmarantz added 2 commits May 23, 2019 14:33
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz dismissed stale reviews from ambuc and ahedberg via e8090f6 May 23, 2019 18:38
@jmarantz
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🔨 rebuilding ci/circleci: tsan (failed build)

🐱

Caused by: a #7046 (comment) was created by @jmarantz.

see: more, trace.

@jmarantz
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7046 (comment) was created by @jmarantz.

see: more, trace.

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.

LGTM w/ small nit. Thank you.

/wait

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 merged commit 47a39f2 into envoyproxy:master May 28, 2019
@jmarantz jmarantz deleted the hystrix-with-stat-names branch May 28, 2019 13:20
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.

4 participants