-
Notifications
You must be signed in to change notification settings - Fork 5.5k
stats: use RE2 and a better pattern to accelerate a single stats tag-extraction RE #8831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
6ffa3ee
add speed & functionality test for using re2 for stats tag matching, …
jmarantz fa3573a
Revert "add speed & functionality test for using re2 for stats tag ma…
jmarantz 42ac3d0
add re2 experiments & speed test for stats init
jmarantz e9b9c88
update a single stat tag-extraction RE to (a) be a better pattern and…
jmarantz d692af2
clean up names of perf categories.
jmarantz fd34b44
format
jmarantz e278b2f
Merge branch 'master' into re-speed-benchmark
jmarantz 7e416e8
Merge branch 'master' into re-speed-benchmark
jmarantz 74abaa4
Merge branch 'master' into re-speed-benchmark
jmarantz a4f297c
format
jmarantz c0a8983
Merge branch 'master' into re-speed-benchmark
jmarantz eba2624
Merge branch 'master' into re-speed-benchmark
jmarantz edf2bb2
Merge branch 'master' into re-speed-benchmark
jmarantz 626a092
Merge branch 're-speed-benchmark' of github.com:jmarantz/envoy into r…
jmarantz b1bfbdc
Merge branch 'master' into re-speed-benchmark
jmarantz 46a3900
review comments (base-class name and enums).
jmarantz 0fa1ca2
format
jmarantz fad62de
Remove extraneous main.
jmarantz 1976786
try to fix clang-tidy problems.
jmarantz 8c79d6c
lint workarounds for benchmark naming convention
jmarantz 5c801a6
Merge branch 'master' into re-speed-benchmark
jmarantz c445533
try to fix clang-tidy and windows compiler errors.
jmarantz 00a0ba1
review comments.
jmarantz 5cf3b48
remaining nits
jmarantz 39783c4
Merge branch 'master' into re-speed-benchmark
jmarantz b5bac7e
review comments
jmarantz f66d732
Merge branch 'master' into re-speed-benchmark
jmarantz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,15 @@ | |
|
|
||
| #include "envoy/stats/tag_extractor.h" | ||
|
|
||
| #include "common/common/regex.h" | ||
|
|
||
| #include "absl/strings/string_view.h" | ||
| #include "re2/re2.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Stats { | ||
|
|
||
| class TagExtractorImpl : public TagExtractor { | ||
| class TagExtractorImplBase : public TagExtractor { | ||
| public: | ||
| /** | ||
| * Creates a tag extractor from the regex provided. name and regex must be non-empty. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update this comment with new parameter
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done; good catch. |
||
|
|
@@ -20,16 +23,16 @@ class TagExtractorImpl : public TagExtractor { | |
| * @param substr a substring that -- if provided -- must be present in a stat name | ||
| * in order to match the regex. This is an optional performance tweak | ||
| * to avoid large numbers of failed regex lookups. | ||
| * @param re_type the regular expression syntax used (Regex::Type::StdRegex or Regex::Type::Re2). | ||
| * @return TagExtractorPtr newly constructed TagExtractor. | ||
| */ | ||
| static TagExtractorPtr createTagExtractor(const std::string& name, const std::string& regex, | ||
| const std::string& substr = ""); | ||
| static TagExtractorPtr createTagExtractor(absl::string_view name, absl::string_view regex, | ||
| absl::string_view substr = "", | ||
| Regex::Type re_type = Regex::Type::StdRegex); | ||
|
|
||
| TagExtractorImpl(const std::string& name, const std::string& regex, | ||
| const std::string& substr = ""); | ||
| TagExtractorImplBase(absl::string_view name, absl::string_view regex, | ||
| absl::string_view substr = ""); | ||
| std::string name() const override { return name_; } | ||
| bool extractTag(absl::string_view tag_extracted_name, TagVector& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
| absl::string_view prefixToken() const override { return prefix_; } | ||
|
|
||
| /** | ||
|
|
@@ -39,19 +42,51 @@ class TagExtractorImpl : public TagExtractor { | |
| */ | ||
| bool substrMismatch(absl::string_view stat_name) const; | ||
|
|
||
| private: | ||
| protected: | ||
| /** | ||
| * Examines a regex string, looking for the pattern: ^alphanumerics_with_underscores\. | ||
| * Returns "alphanumerics_with_underscores" if that pattern is found, empty-string otherwise. | ||
| * @param regex absl::string_view the regex to scan for prefixes. | ||
| * @return std::string the prefix, or "" if no prefix found. | ||
| */ | ||
| static std::string extractRegexPrefix(absl::string_view regex); | ||
|
|
||
| /** | ||
| * Adds a new tag for the current name, returning a reference to the tag value. | ||
| * | ||
| * @param tags the list of tags | ||
| * @return a reference to the value of the tag that was added. | ||
| */ | ||
| std::string& addTag(std::vector<Tag>& tags) const; | ||
|
|
||
| const std::string name_; | ||
| const std::string prefix_; | ||
| const std::string substr_; | ||
| }; | ||
|
|
||
| class TagExtractorStdRegexImpl : public TagExtractorImplBase { | ||
| public: | ||
| TagExtractorStdRegexImpl(absl::string_view name, absl::string_view regex, | ||
| absl::string_view substr = ""); | ||
|
|
||
| bool extractTag(absl::string_view tag_extracted_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
|
|
||
| private: | ||
| const std::regex regex_; | ||
| }; | ||
|
|
||
| class TagExtractorRe2Impl : public TagExtractorImplBase { | ||
| public: | ||
| TagExtractorRe2Impl(absl::string_view name, absl::string_view regex, | ||
| absl::string_view substr = ""); | ||
|
|
||
| bool extractTag(absl::string_view tag_extracted_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
|
|
||
| private: | ||
| const re2::RE2 regex_; | ||
| }; | ||
|
|
||
| } // namespace Stats | ||
| } // namespace Envoy | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider options for avoiding duplicating a lot of the logic/comments between here and the std::regex code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I messed around a bit and was only really able to profitably factor out the code to add a tag to the tags array. There were enough subtle difference in all the other parallels that it didn't seem worth it to refactor. LMK if you have other ideas.