Re: [boost] Re: Re: Profiling Library suggestion

From: christopher diggins (cdiggins_at_[hidden])
Date: 2005-02-11 17:55:26

----- Original Message ----- 
From: "Caleb Epstein" <caleb.epstein_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Friday, February 11, 2005 5:41 PM
Subject: Re: [boost] Re: Re: Profiling Library suggestion

> On Fri, 11 Feb 2005 16:34:36 -0500, Preston A. Elder
> <prez_at_[hidden]> wrote:
>
>> BTW, I'm wondering why you don't use something like this as the basis for
>> your high-resolution timer:
>>
>> boost::posix_time::ptime get_highres_time()
>
> One subtle difference is that the current implementation uses
> CPU-cycle timings (via std::clock or Win32 QueryPerformanceCounter)
> and not wall-clock timings.  Using the xtime_get approach would give
> you a wall-clock based timer which may not be what you want.

I don't fully appreciate the differences. Can you provide a more detailed 
pro and con for the two methods?

> Christopher: a couple of comments on the current implementation of the
> basic_profiler:
>
> * std::map and std::string are used unqualified and w/o headers being 
> included

Noted.

> * string arguments to methods should be passed by const ref

Yes.

> * There should be a public elapsed() method to extract the elapsed
> time from a profiler. Perhaps it should just be public?

Yes.

> * basic_profiler::restart invokes logging_policy::on_restart which is
> not implemented (should this be on_resume?)

I should add on_restart.

> * basic_profiler::restart, ::resume and ::pause all call methods on an
> undefined object "timer".  Should be "t".

Noted.

> * basic_profiler::pause calls (when fixed) t.pause which is not 
> implemented.

Noted.

> * In the private section of basic_profiler, the typename in "typename
> timer_t t" is (I think) spurious.

I'll look into it.

> * The basic_profiler ctor initializes members in a different order
> than they are declared in the class

Does that matter?

> Might I also suggest that the basic_profiler.stop method not zero out
> the elapsed time.  Leave it to restart to do that.  Think of it like a
> stop-watch.

Yes.

> -- 
> Caleb Epstein
> caleb dot epstein at gmail dot com

Excellent points,
Thank you very much!

Christopher Diggins