Re: Profiler class, suggestions and comments welcome
On Aug 19, 4:41 pm, Francesco <entul...@gmail.com> wrote:
Hence I created a profiler class that automatically logs the
start/ stop times upon construction/destruction.
Here is my profiler class. A couple of questions right after it:
Just a note: most functions will take so little time that the
results of measuring one execution isn't reliable. And what
your class does isn't so much profiling, but benchmarking.
Which has it's own set of problems. (I'd recommend your having
a look at my BenchHarness class, but my site still isn't
working. I'd say next week-end, but I'm in the middle of
moving, which takes up a lot of time.)
-------
// combining header and implementation for easy copy&paste
// header (double-inclusion check snipped out)
#include <string>
namespace profiler {
class Profile {
public:
Profile(std::string = "");
~Profile();
private:
std::string id;
};
std::string report();
}
// implementation
#include <ctime>
#include <map>
#include <sstream>
// profiler's header inclusion snipped
struct time_pair {
clock_t start;
clock_t stop;
};
typedef std::map<std::string, time_pair> time_map_type;
time_map_type time_map;
profiler::Profile::Profile(std::string s) {
id = s;
clock_t& rct = time_map[id].start;
rct = clock();
}
profiler::Profile::~Profile() {
clock_t ct = clock();
time_map[id].stop = ct;
}
std::string profiler::report() {
time_map_type::const_iterator iter;
std::stringstream res;
for(iter = time_map.begin(); iter != time_map.end(); ++iter) {
res << iter->first << "\n" << "Start: ";
res << iter->second.start << ", stop: ";
res << iter->second.stop << ", total: ";
res << (iter->second.stop - iter->second.start) << "\n";
}
return res.str();
}
Both time_map (and it's type) and the function report should be
static members of profiler. I'd also make the time_pair type a
member (and give it a more significant name, something like
Interval).
(And while I'm at it, your naming convention doesn't seem to
regular: some types start with capital letters, others don't.
As a general rule, it's probably a good idea to distinguish
between types and things like variables and functions in the
naming convention---starting types with a capital letter is
probably the most frequent convention. I use the same naming
convention for namespaces, since they tend to occur in similar
contexts as class names, e.g. to the left of the :: operator.)
-------
I use it in this way:
-------
void f() {
profiler::Profile p("f()");
// ... code to be profiled ...}
-------
And here are the questions:
- Why must I set "~Profile()" as public? (apart from the fact that the
compiler refuses to compile it if I set it to private or protected)
The simple reason is because the standard says so. But it's
logical---you're calling the destructor in the non-member
function f. In general, when defining a variable with static,
automatic or temporary scope, the destructor must be accessible
at the site of the definition.
Shouldn't it be something to be left out from the public
interface so that users can not mess with it? <--- academic
question, I don't have such issues (unfortunately...)
What do you mean by: "that users cannot mess with it"? If the
user constructs an instance, they have to be able to destruct
it (with a few exceptions).
- Notice how I used an additional variable and a reference to
avoid including the map lookup time into the logged time, in
the ctor and the dtor. Is this a smart move or is it just
paranoia?
It's one solution. I'd probably have had Profile contain a
private (non-static) time_pair, and only inserted in the map in
the destructor. But given that for many smaller functions, the
lookup in the map could easily be longer than the function, you
obviously have to move that out of the interval being timed.
--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34