Optimization to Skip this entire family #2105
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. |
urmez
changed the title
|
can you add tests please? and run the formatter. |
src/benchmark_register.cc
Outdated
| // Check if the filter spec starts with the family name | ||
| // or if the family name appears as a substring in the spec | ||
| family_could_match = spec.find(family->name_) != std::string::npos || | ||
| spec == "." || |
There was a problem hiding this comment.
The . special case should probably be checked first.
src/benchmark_register.cc
Outdated
|
|
||
| // Optimization: Quick check if family name could match the filter | ||
| // before iterating through all args combinations | ||
| bool family_name_disabled = family->name_.rfind(kDisabledPrefix, 0) == 0; |
There was a problem hiding this comment.
Why rfind() though, instead of find()?
That being said i think it might make sense to define a helper function string_has_prefix() for this...
There was a problem hiding this comment.
No rfind now. let me know what you think of latest code.
The optimization only works for literal string to skip the complete family and all its args spec which is regex, so a pre-check is needed to make sure to skip only on a litral string match and not when its regex. Added a unit test to measure the speedup
@dmah42 "run the formatter" : explain or point to the documentation on what and how to run this |
|
There is a clang format CI job failing. A clang format config file is located in the root of the project. Running |
| // Verify optimization provides at least 5x speedup | ||
| if (speedup < 5.0) { | ||
| std::cerr << "ERROR: Expected at least 5x speedup, got " << speedup << "x\n"; | ||
| std::cerr << "Optimization may not be working correctly!\n"; | ||
| return -1; | ||
| } | ||
| } else { | ||
| std::cerr << "ERROR: Literal filter completed too fast to measure accurately\n"; | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
It is not a good idea to make tests depend on host machie performance.
| if (!is_negative_filter && !family_contains_literal) { | ||
| // Positive filter: family name doesn't contain literal, skip family | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Won't this errneously skip BM_Family/arg1/arg2 given filter BM_Family/arg ?
3 participants
No description provided.