----- Original Message ----- From: "Gennadiy Rozental" <gennadiy.rozental@thomson.com>
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