[squid-dev] [RFC][CODE] RAII profiler

[Kinkie]

Hi,
  the attached patch implements a profiler API fashioned after the RAII
pattern.
It does nothing to attack the c API or the implementation, just adds on top
of it a Profiler class to save the caller the hassle of having to track all
exit paths from a function.
As an example, it implements the new API for the HttpHeader::parse method.

BTW: there is an issue in the profiler cachemgr report - it segfaults for
me on mac.
I'll try to check it out on Linux in the next few days.

-- 
Kinkie


profiler.patch
Description: Binary data
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] [RFC][CODE] RAII profiler

[Amos Jeffries]

On 11/08/2015 10:52 a.m., Kinkie wrote:
> Hi,
>   the attached patch implements a profiler API fashioned after the RAII
> pattern.
> It does nothing to attack the c API or the implementation, just adds on top
> of it a Profiler class to save the caller the hassle of having to track all
> exit paths from a function.
> As an example, it implements the new API for the HttpHeader::parse method.

Thank you or picking this up.

Please move the new class and #define outside the USE_XPROF_STATS
blocks. Use USE_XPROF_STATS wrapper to switch the class internals about
instead.
 That makes the class API guaranteed to be consistent, or where
inconsistent obviously so. And halves the LOC.


> 
> BTW: there is an issue in the profiler cachemgr report - it segfaults for
> me on mac.
> I'll try to check it out on Linux in the next few days.
> 

Amos

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] [RFC][CODE] RAII profiler

[Alex Rousskov]

On 08/10/2015 04:52 PM, Kinkie wrote:
> Hi,
>   the attached patch implements a profiler API fashioned after the RAII
> pattern.
> It does nothing to attack the c API or the implementation, just adds on
> top of it a Profiler class to save the caller the hassle of having to
> track all exit paths from a function.
> As an example, it implements the new API for the HttpHeader::parse method.


I would prefer a larger-context diff, but it looks like you are on the
right track overall. Here are a few suggestions:

Instead of changing a lot of old code to install the new profiler
instead of the old one, rewrite PROF_start() to install the new
profiler. Some hand-edits will still be necessary, but not too many.
PROF_stop() will do nothing and calls to it can be eventually removed
when the old code is modified for some other reason.

PROF_profiler() should not require any additional declarations by the
user. Rework to make "auto p" optional (if very easy) or impossible
(otherwise). The macro can combine a hard-coded prefix with __LINE__ to
form unique local variable names.

Ideally, adding a profiling probe should not require the user to type
the probe name *by default*. The new macro can combine a hard-coded
prefix with __FUNCTION__ (or similar) and possibly __LINE__ to form
context-friendly variable names. Do we need to support compilers that do
not set __FUNCTION__ or some such?

If possible, PROF_profiler() should not insert any code when profiling
is disabled, not even an empty constructor call.

I would also avoid ALL_caps() names for new code. Something like
ProfileMe() or Profile(Here) would work better IMO.


You may find some useful ideas in my 2011 profiling patch at
http://www.squid-cache.org/mail-archive/squid-dev/201101/0067.html
For example, that dirty patch implements RAII context profilers via
ProfileScope() and uses some of the tricks mentioned above.


HTH,

Alex.

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev