[boost] Re: Re: Re: Boost Profiler Proposal

From: Gennadiy Rozental (gennadiy.rozental_at_[hidden])
Date: 2005-02-07 17:15:59

> > Well, I've read you docs and code. Here is what I think:
> >
> > Once I started looking through proposed design it immodestly stroke me
> > that
> > you are making most common mistake in policy based designs (PBD):
putting
> > everything into one single policy. IMO PBD is not about providing
complete
> > implementation as template parameter (at least is not only about it).
But
> > mostly about finding small reusable and orthogonal! policies that in
> > combination deliver flexibility and power. This is the reason why your
> > policy are templates and why you need to implement counting, reporting
and
> > counting_and_reporting policies. These are orthogonal parts of profiler
> > functionality. No need to combine them into single policy.
>
> Good point, I agree that the timer_type should be extracted. However both
> reporting and collecting policies model the same concept,

No they model completely different notions:

what to collect
and
how to report

> they react to when the profiler object is destroyed.

Not necessarily. Both report and collection functionality may need to be
invoked at other point explicitly

> Separating them does not seem necessary to me.

Now to implement custom logging into Boost.Test log I need to repeat
counting logic, is it?

> I think that particular point may be more of an issue of personal
> style, because overall it does not change the behavior of the class.

It does. It makes inflexible.

> >  Also I believe from my experience that profiler model you chose is way
> > too
> > simple for real-life needs. Here is specific profiling modes I saw:
> >
> > 1. Take piece of code and wrap into timer. You cover this case
> > 2. Take piece of code and time an average execution time during multiple
> > code invocation. You model couldn't do that
>
> Actually it can. The reporting_policy stores elapsed_total and entry_count

Actually you can't. Not without changing public interfaces.

> which can be used to construct averages. I have improved the
> collecting_policy to generate detailed reports:
>
>   struct counted_sum : pair<int, double> {
>     counted_sum() : pair<int, double>(0, 0) { }
>     counted_sum(int x, double y) : pair<int, double>(x, y) { }
>     void operator+=(double x) {
>       first++;
>       second += x;
>     }
>   };
>
>   typedef std::map<std::string, counted_sum> stats_map;
>
>   struct collecting_policy
>   {
>     static stats_map stats;

And what if I have two independent profiling point?

> > 3. Frequently code you want to profile is "interrupted" with islands of
> > code
> > that you are not interested in. You need an ability to do accumulation
>
> I would prefer to leave this to the user when they generate their report.
I
> see no real obstacle for them to do the subtraction of stats.

I don't know what substraction you mean. It's rather addition. And in any
case I believe it's component responsibility.

{
profiler p;

// some code

p.stop();

// some other code we do not want to profile

p.resume();

// continue profiling

p.stop()

} // here we print report

I expect report to be print only once

> > 4. Hierarchical checking points. Frequently you want to be able to see
whe
> > whole picture and easily add check points somewhere inside. In a result
> > you
> > want to see time from point A to point B then from point B to point C,
> > combined time from A to C and so on.
>
> See above.

See what?

> I have posted a new version of the code at
> http://www.cdiggins.com/profiler/profile2.hpp which separates the timer
> policy, and allows the generation of reports after execution. How do you
> feel about the new design, does it go far enough in your mind, or do you
> still want significantly more functionality?

What I described is only basics that any decent profiler component should
provide. There are some other uncovered areas.

> I feel relatively strongly that
> the library should simply provide the data to the user, and provide a
> minimal default functionality in terms of generating reports, and leave
the
> rest up to the user.

Combining independent notions into single entity doesn't make your design
simple. In fact it makes it harder to reuse and extend.

> Thanks for your suggestions and feedback,
> Christopher Diggins

Gennadiy

P.S. BTW your code is inaccessible