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?
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
What I described is only basics that any decent profiler component should provide. There are some other uncovered areas. 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