Copilot commented on code in PR #3101:
URL: https://github.com/apache/brpc/pull/3101#discussion_r2370864723
##########
src/brpc/details/jemalloc_profiler.cpp:
##########
@@ -166,6 +166,13 @@ static int JeProfileReset(size_t lg_sample) {
return 0;
}
+// https://github.com/jemalloc/jemalloc/blob/5.3.0/bin/jeprof.in#L211-L222
+static std::unordered_set<std::string> g_extra_options_set = {
+ "inuse_space", "inuse_objects", "alloc_space",
+ "alloc_objects", "show_bytes", "drop_negative",
+ "total_delay", "contentions", "mean_delay"
+};
Review Comment:
Consider using a `const` qualifier for `g_extra_options_set` since it's
initialized once and never modified. This makes the intent clearer and may
enable compiler optimizations.
##########
src/brpc/details/jemalloc_profiler.cpp:
##########
@@ -228,13 +235,15 @@ void JeControlProfile(Controller* cntl) {
}
const std::string process_file(process_path, len);
- std::string cmd_str = jeprof + " " + process_file + " " + prof_name;
+ std::string cmd_str = butil::string_printf(
+ "%s %s %s", jeprof.c_str(), process_file.c_str(), prof_name.c_str());
// https://github.com/jemalloc/jemalloc/blob/5.3.0/bin/jeprof.in#L211-L222
// e.g: inuse_space, contentions
const std::string* uri_extra_options = uri.GetQuery("extra_options");
- if (uri_extra_options != nullptr && !uri_extra_options->empty()) {
- cmd_str += " --" + *uri_extra_options + " ";
+ if (uri_extra_options != nullptr && !uri_extra_options->empty() &&
+ g_extra_options_set.find(*uri_extra_options) !=
g_extra_options_set.end()) {
Review Comment:
[nitpick] Consider using `g_extra_options_set.count(*uri_extra_options)`
instead of `find() != end()` for checking set membership. It's more readable
and potentially more efficient for this use case.
```suggestion
g_extra_options_set.count(*uri_extra_options)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
