-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
avoid regex 'parsing' format strings every time #2164
Conversation
void split(); | ||
|
||
public: | ||
FormatString() = default; |
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.
shouldn't need this but got some confusing template errors. Will try to remove it
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.
Don't see a way to avoid this, unless we change the vector<tuple<FormatString, vector<Fields>>
into something with a custom serializer that does it correctly. Unless I'm doing something wrong.
Having this also keeps the serializer for this class easy.
02bc547
to
5a662e9
Compare
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.
Just one comment, LGTM otherwise.
@@ -6,70 +6,6 @@ | |||
|
|||
namespace bpftrace { | |||
|
|||
std::string verify_format_string(const std::string &fmt, std::vector<Field> args) |
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.
Should be also removed from src/printf.h
.
ac29327
to
4b62cc3
Compare
bpftrace::format spends about 60% of its time in the regex_iterator we use to split the string. The FormatString type avoids this by splitting the string on first use and storing the result for re-use. I've kept the code mostly as is, just moving and minor modifications. The whole output code can do with a refactoring but this should be a nice performance gain. Test case is running opensnoop.bt while running `find /` in another terminal: original: bpftrace::perf_event_printer spent 75% of the time in format new: bpftrace::perf_event_printer spent 15% of the time in format
bpftrace::format spends about 60% of its time in the regex_iterator we
use to split the string.
The FormatString type avoids this by splitting the string on first use
and storing the result for re-use.
I've kept the code mostly as is, just moving and minor modifications.
The whole output code can do with a refactoring but this should be a
nice performance gain.
Test case is running opensnoop.bt while running
find /
in anotherterminal:
original: bpftrace::perf_event_printer spent 75% of the time in format
new: bpftrace::perf_event_printer spent 15% of the time in format
Checklist
man/adoc/bpftrace.adoc
and if needed indocs/reference_guide.md
CHANGELOG.md