Re: Profiler class, suggestions and comments welcome
Francesco wrote:
Hi everybody,
with this very post I'm starting my first thread into this NG.
I know that my behavior so far has been [..]
If you didn't write this, nobody would notice. Everybody here couldn't
care less about your behavior unless it's against the spirit of the
forum or the rules of netiquette.
So then, here is the point.
I lack the knowledge to properly use debuggers and profilers that come
with Code::Blocks.
Don't worry. I don't even know what "Code::Blocks" is.
> Actually, I'm not even sure about whether it
includes a profiler... never mind.
Hence, I usually debug my code by printing out variable values to the
console and the alike.
About measuring the performance of my code, I used functions like
"StartTimer(string timername)" and "StopTimer(string timername)".
After a short while I got tired of calling such functions at the begin
and at the end of the functions I was profiling - along with having to
recall the string IDs or creating local strings to avoid recalling
them.
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:
-------
// combining header and implementation for easy copy&paste
// header (double-inclusion check snipped out)
#include <string>
namespace profiler {
class Profile {
public:
Profile(std::string = "");
For the sake of performance (and with a profiler it's kind of important,
wouldn't you say?) you should *never* pass objects of class type by
value. *Always* prefer passing by a const ref, unless there is a strong
reason not to.
~Profile();
private:
std::string id;
Since your individual object survives only until the end of the scope in
which it was created, consider storing the start clock value in it,
instead of in a separate table. Then in the destructor calculate the
difference and store it in the table (also see below for more
recommendations).
};
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;
*Always* prefer initialisation (using the initialiser list) over
assignment. There is a FAQ about that. Have you actually read the FAQ?
clock_t& rct = time_map[id].start;
rct = clock();
}
So, your function here could be written as
profiler::Profile::Profile(std::string const & s)
: id(s)
{
time_map[id].start = clock();
}
profiler::Profile::~Profile() {
clock_t ct = clock();
time_map[id].stop = ct;
Since you don't really know in what order the operands of an assignment
expression are going to be evaluated, it is probably a good idea to
enforce the order like you did here.
}
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();
}
-------
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?
Because a local object 'p' is created *AND* destroyed from a scope that
has no special access to your 'Profile' class.
> (apart from the fact that the
compiler refuses to compile it if I set it to private or protected)
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...)
Destructors aren't usually called explicitly. It's object creation and
their disposal that are controlled by the user.
- 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?
Pat yourself on the back. After you have stood in the corner for not
passing arguments by const ref.
I don't have any other specific question about this very simple piece
of code, nonetheless I'll be happy to read your comments and your
advices on any subject - naming style included.
See above.
Also, consider that the results of profiling of any particular function
are overridden by a subsequent calls, so you might consider improving
your system to (a) count the number of calls to that particular function
and (b) accumulate the overall time spent in it (instead of recording
start-stop ticks):
struct timing_result {
int hit_count;
clock_t total_time_spent;
};
Some time later I'll ask for your advice about something more
interesting (more interesting to me: a simulation of ants harvesting
food around their nest). I'm not so sure, though. It will involve
posting the SDL interface code, asking questions about generic OOP and
about ants' behavior, hence I'm not sure this will be the right place
to post it - assuming I'll be ever brave enough to show my
"abomination" to the world ;-)
Remember that we often do not have time to delve into the intricacies of
your model domain. If you have a language problem or a question, we're
happy to help. Modelling and OOA/OOD are discussed in comp.object.
V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask