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

From: christopher diggins (cdiggins_at_[hidden])
Date: 2005-02-07 19:38:24

----- Original Message ----- 
From: "Gennadiy Rozental" <gennadiy.rozental_at_[hidden]>
>> 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

I said "concept" not notion, referring to the fact that they were triggered 
under precisely the same circumstances and had the same function signatures.

>> 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

They couldn't be in my design. And I currently am entirely unconvinced of 
the need to further complicate the design.

>> 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?

IMO repeating counting logic is trivial and does not neccessarily warrant a 
policy.

>> 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.

Not in the case of the current design at 
http://www.cdiggins.com/profiler/profiler2.hpp . Adding multiple policies to 
an RAII class which only calls 1 function would be needlessly complicating 
the design.

>> >  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.

You seem to be confusing structs with classes. The fields are public!

In the old version ( http://www.cdiggins.com/profiler/profiler.hpp ) you 
would have written:

double avg = collecting_policy::elapsed_totals["name"] / 
collecting_policy::entry_counts["name"];

With the new version ( http://www.cdiggins.com/profiler/profiler2.hpp ) the 
average can now be computed as:

sum_count sc = collecting_policy::stats["name"];
avg = sc.first() / sc.second();

>> 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?

Then they should have different names. Each profile is labelled according to 
a name, so it either occupies a unique location in the map, or it overlaps 
with another profile, if that is the desire.

>> > 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.

There are two ways to compute an island:

Version 1)
Subtraction method:

{
  profile p1("name1")
  {
    profile p2("name2");
    // island
  }
}

cout << collecting_policy["name1"].first - collecting_policy["name2"].first 
<< endl;

Version 2)
Addition method:

{
  profile p1("name1");
}
// island
{
  profiler p2("name2");
}

cout << collecting_policy["name1"].first + collecting_policy["name2"].first 
<< endl;

Version 3)
Shared Named method :

{
  profile p1("name1");
}
// island
{
  profiler p1("name1");
}

cout << collecting_policy["name1"].first;

> And in any
> case I believe it's component responsibility.

I disagree.

> {
> 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

stop() and resume() are not part of the design.

You would write the above code as:

{
  {
    basic_profiler<collecting_policy> p("profile1");
    // some code
  }
  // some other code we do not want to profile
  {
    basic_profiler<collecting_policy> p("profile1");
    // continue profiling
  }
  collecting_policy::generate_report();
  }
}

CD