Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
LebedevRI
left a comment
LebedevRI
left a comment
There was a problem hiding this comment.
I think inherit is too undecipherable,
it should probably be benchmark_perf_counters_for_all_threads / benchmark_perf_counters_all_threads or something..
| " [--benchmark_counters_tabular={true|false}]\n" | ||
| #if defined HAVE_LIBPFM | ||
| " [--benchmark_perf_counters=<counter>,...]\n" | ||
| " [--benchmark_perf_counters_inherit={true|false}]\n" // <--- Add this |
There was a problem hiding this comment.
remove the AI generated comment please
|
|
||
| PerfCounters PerfCounters::Create( | ||
| const std::vector<std::string>& counter_names) { | ||
| const std::vector<std::string>& counter_names, bool inherit) { |
There was a problem hiding this comment.
boolean parameters are notoriously hard to track as the callsite becomes opaque : https://abseil.io/tips/94
| @@ -311,20 +312,21 @@ bool PerfCounters::Initialize() { return false; } | |||
| bool PerfCounters::IsCounterSupported(const std::string&) { return false; } | |||
|
|
|||
| PerfCounters PerfCounters::Create( | |||
There was a problem hiding this comment.
i think this would be clearer as two methods: Create and CreateInherited (or whatever naming you land on)
3 participants
First Contribution I have made for open source, Some formatting issues might be present
Fixing this issue ([FR] Allow modification of perf attributes #1897)
I added an optional Command Line Argument
-- benchmark_perf_counters_inherit (Either True or False) and True by Default
True = Benchmark Parent and Child Processes
False = Benchmark just the parent process
True if unspecified