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

From: christopher diggins (cdiggins_at_[hidden])
Date: 2005-02-07 16:29:25

----- Original Message ----- 
From: "Gennadiy Rozental" <gennadiy.rozental_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Monday, February 07, 2005 1:57 PM
Subject: [boost] Re: Re: Boost Profiler Proposal

[snip]
> Well, I've read you docs and code. Here is what I think:
>
> Once I started looking through proposed design it immideatly 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, they react to when 
the profiler object is destroyed. Separating them does not seem neccessary 
to me. I think that particular point may be more of an issue of personal 
style, because overall it does not change the behaviour of the class.

>  Also I believe from my expirience 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 invokation. You model couldn't do that

Actually it can. The reporting_policy stores elasped_total and entry_count 
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;
    static void on_profiled(const char* name, double dSec, double dMin, 
double dMax) {
      stats[name] += dSec <= dMin ? 0.0 : dSec;
    }
    static void generate_report() {
      cout
        << "profile name" << '\t'
        << "total elapsed" << '\t'
        << "entry count" << '\t'
        << "average" << endl;

      for (stats_map::iterator i=0; i != stats.end(); i++)
      {
        string sName = i->first;
        int nCount = i->second.first;
        double dTotal = i->second.second;
        double dAvg = dTotal / nCount;
        cout
          << sName << '\t'
          << dTotal << '\t'
          << nCount << '\t'
          << dAvg << endl;
      }
    }
  };

  stats_map collecting_policy::stats;

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

> 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,
> compined time from A to C and so on.

See above.

> Here is how I think policies could be designed:
>
> 1. TimerPolicy
>  Responcible for "timing implementation", including timer, it's resolution
> e.t.c. Possible incarnations: BoostTimerPolicy, HighResolutionTimerPolicy,
> WallTimerPolicy.
> 2. CollectionPolicy
>  Responcible for "what and how to collect" (elapsed time in one run,
> average time in multiple runs, profiler name, location, relations). 
> Possible
> incarnations: SingleRunCollector. MultirunCollector, GlobalCollector 
> e.t.c.
> This policy may need to be separated into two.
> 3. LoggingPolicy
>  Responcible for "how to report". Here where Boost.Test integration kicks
> in. Possible incarnations: OstreamLogger, StdoutLogger, BoostTestLogger
> e.t.c.
> 4. ReportingPolicy
>  Responcible for "when to report". Possible incarnations: ScopedReporter,
> ExplicitReporter, ReportNone (to be used with GlobalCollector that perform
> reporting itself)
>
> and profiler definition would look like this:
>
> template<typename TimerPolicy,typename CollectionPolicy,typename
> LoggingPolicy,typename ReportingPolicy>
> class profiler :  public TimerPolicy, public CollectionPolicy, public
> LoggingPolicy, public ReportingPolicy {
> ....
> };

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

Thanks for your suggestions and feedback,
Christopher Diggins