Re: profiler implementation

[John W. Eaton]

On 10-Aug-2011, Daniel Kraft wrote:

| On 08/10/11 19:29, John W. Eaton wrote:
| > Is the nested "enter" class really needed?  Wouldn't it be possible
| > to simply write
| > 
| > { if (Vprofiling) profile_data_accumulator pda (name);
| > 
| > ... }
| 
| Again, I think that would not work because of the scoping of pda.

OK, I see the problem.

| And
| further, we don't want to create a new data_accumulator -- we just want
| a object which manages stack unwinding.  The data_accumulator must
| persist, since it has to store also other function entries/exits and
| collect the data.

Maybe the naming above is bad, I think the idea is basically the
same.  You currently have

  {
    profile_data_accumulator::enter pe (profiler,
                                        profiler_name ());
    cmd_list->accept (*current_evaluator);
  }

which creates a temporary object for the scope.  With what I was
suggesting, we would do the same, but the temporary object would
interact with the global state instead of having some global object
passed to it.

In any case, since there is some time pressure, I'd say don't worry
about restructuring these classes now.  We can continue to discuss
this later, after Google's deadline.

| Thus yes, I don't see how we could do without *two* different classes.
| 
| > This could even be written as
| > 
| > #define BEGIN_PROFILE_BLOCK \ { \ if (Vprofiling) \ 
| > profile_data_accumulator pda (name);
| > 
| > #define END_PROFILE_BLOCK \ }
| > 
| > BEGIN_PROFILE_BLOCK ... END_PROFILE_BLOCK
| > 
| > to make it less likely that someone who doesn't understand the way 
| > the profiler works will come along later and remove the braces.
| 
| *That* is something I could actually do; if you think that this would be
| worthwhile and not obfuscate the code (because I think a lot of people
| don't really like such "code generating" macros; although I use them
| some times and don't think they're too bad), I'll happily do that.

We already have a number of them for cases like this, so I don't think
this is much of a problem.  I'd rather mark the scope this way than do
it with simple braces.  It seems much less likely that someone would
remove these macros accidentally.  So I'd say go ahead and use macros
like this.

Thanks,

jwe