Re: profiler implementation

[John W. Eaton]

On 10-Aug-2011, Jordi Gutiérrez Hermoso wrote:

| You see this as code that has to change and thus rigidity in the code.
| I see this as flexibility. If we ever decide to use a different
| profiler to profile a different kind of Octave function, we can use
| another global object to profile that other kind of Octave function.

I think you could get this flexibility without having to change all
the places where the profiler is called if you used a different
structure.  For example, instead of having

  profile_data_accumulator::enter pe (profiler, profiler_name ());

where profiler is the actual profiler object, you might use

  profile_data_accumulator::enter pe (current_profiler (), ...)

But then, why not just have the "profile_data_accumulator" object
handle this detail instead of exposing it to every use of the object.
Or should the selection of the profiler that will be used the job of
every user of the profiler objects?

Is the nested "enter" class really needed?  Wouldn't it be possible to
simply write

  {
    if (Vprofiling)
      profile_data_accumulator pda (name);

    ...
  }

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.

| The singleton pattern just seems to me like fluff that you wrap around
| a global object.
|
| > If you just want a single accumulator object,
| 
| But we might not. And while we do want one, the above works fine.
| 
| Global objects are fine; I see no point in wrapping a global object
| with redundant get_instance() and do_thing() functions.

The singleton pattern can easily be modified so that get_instance
manages a list of possible instances, not just a single one.  And then
the code that uses the "singleton" doesn't have to change at all.
You simply need to change the way the get_instance method allocates
and selects the particular instance to use.

jwe