Using decorator for power monitoring.

Using decorator for power monitoring.

R=tonyg@chromium.org,jeremy@chromium.org

[qsr, 2014-04-09 11:30:06]

Tony, Jeremy -> I'm working on adding a power metric for load.

 Doing this, I found that we had to declare and call the power metrics and each
and every page measurement that wants to use it, which is kind of cumbersome.

 What do you think about using decorators to ease adding power (or if we
continue that way, any other metrics) to a page set?

[qsr, 2014-04-09 12:26:05]

Oh yes also, not ready to review, just want an answer on the idea, will clean it
up, and use it on all measurement if we agree...

[dtu, 2014-04-09 18:04:35]

On 2014/04/09 12:26:05, qsr wrote:
> Oh yes also, not ready to review, just want an answer on the idea, will clean
it
> up, and use it on all measurement if we agree...

Are you familiar with MixIns? That would be a cleaner way of doing the same
thing.
(A MixIn is just using multiple inheritance to override/wrap the class's
methods.)

[qsr, 2014-04-10 07:39:46]

On 2014/04/09 18:04:35, dtu wrote:
> On 2014/04/09 12:26:05, qsr wrote:
> > Oh yes also, not ready to review, just want an answer on the idea, will
clean
> it
> > up, and use it on all measurement if we agree...
> 
> Are you familiar with MixIns? That would be a cleaner way of doing the same
> thing.
> (A MixIn is just using multiple inheritance to override/wrap the class's
> methods.)

 I didn't know python had mixin... And I must be missing something when reading
the doc I do not see how I can do what I want with it.

 If I make my measurement extends my mixin, I will have to call super() on every
method for this to work, while by using my decorator, I can have the metric
decorator choose with method it overrides and be responsible for then calling
the original method. Does it make sense or did I miss something obvious?

[dtu, 2014-04-10 19:52:43]

On 2014/04/10 07:39:46, qsr wrote:
> On 2014/04/09 18:04:35, dtu wrote:
> > On 2014/04/09 12:26:05, qsr wrote:
> > > Oh yes also, not ready to review, just want an answer on the idea, will
> clean
> > it
> > > up, and use it on all measurement if we agree...
> > 
> > Are you familiar with MixIns? That would be a cleaner way of doing the same
> > thing.
> > (A MixIn is just using multiple inheritance to override/wrap the class's
> > methods.)
> 
>  I didn't know python had mixin... And I must be missing something when
reading
> the doc I do not see how I can do what I want with it.
> 
>  If I make my measurement extends my mixin, I will have to call super() on
every
> method for this to work, while by using my decorator, I can have the metric
> decorator choose with method it overrides and be responsible for then calling
> the original method. Does it make sense or did I miss something obvious?

Ah, you're right, there's a tradeoff I didn't realize.
Using MixIns also reduces the boilerplate in the Decorator/MixIn itself.

Instead of:
  originalCustomizeBrowserOptions = original_class.CustomizeBrowserOptions
  def newCustomizeBrowserOptions(self, options):
    originalCustomizeBrowserOptions(self, options)
    power.PowerMetric.CustomizeBrowserOptions(options)
  original_class.CustomizeBrowserOptions = newCustomizeBrowserOptions
You have:
  def CustomizeBrowserOptions(self, options):
    super(PowerMetricMixIn, self).CustomizeBrowserOptions(options)
    power.PowerMetric.CustomizeBrowserOptions(options)

The tradeoff is that yes, if the measurement overrides methods, you'll need to
call super. But a lot of measurements (e.g. smoothness.py) won't need to define
any methods.

[qsr, 2014-04-11 06:57:31]

On 2014/04/10 19:52:43, dtu wrote:
> On 2014/04/10 07:39:46, qsr wrote:
> > On 2014/04/09 18:04:35, dtu wrote:
> > > On 2014/04/09 12:26:05, qsr wrote:
> > > > Oh yes also, not ready to review, just want an answer on the idea, will
> > clean
> > > it
> > > > up, and use it on all measurement if we agree...
> > > 
> > > Are you familiar with MixIns? That would be a cleaner way of doing the
same
> > > thing.
> > > (A MixIn is just using multiple inheritance to override/wrap the class's
> > > methods.)
> > 
> >  I didn't know python had mixin... And I must be missing something when
> reading
> > the doc I do not see how I can do what I want with it.
> > 
> >  If I make my measurement extends my mixin, I will have to call super() on
> every
> > method for this to work, while by using my decorator, I can have the metric
> > decorator choose with method it overrides and be responsible for then
calling
> > the original method. Does it make sense or did I miss something obvious?
> 
> Ah, you're right, there's a tradeoff I didn't realize.

 There is another, and one that I need, I need to be able to decorate the method
before and after the call, which I do not see how to do with mixins.

> Using MixIns also reduces the boilerplate in the Decorator/MixIn itself.
> 
> Instead of:
>   originalCustomizeBrowserOptions = original_class.CustomizeBrowserOptions
>   def newCustomizeBrowserOptions(self, options):
>     originalCustomizeBrowserOptions(self, options)
>     power.PowerMetric.CustomizeBrowserOptions(options)
>   original_class.CustomizeBrowserOptions = newCustomizeBrowserOptions
> You have:
>   def CustomizeBrowserOptions(self, options):
>     super(PowerMetricMixIn, self).CustomizeBrowserOptions(options)
>     power.PowerMetric.CustomizeBrowserOptions(options)
> 
> The tradeoff is that yes, if the measurement overrides methods, you'll need to
> call super. But a lot of measurements (e.g. smoothness.py) won't need to
define
> any methods.

 Well, a little helper function can do a lot for the boiler plate:

 def _replace(replacement):
    """Replace the the function of original_class whith the same name as
    replacement by replacement.
    """
    setattr(original_class,
            replacement.__name__,
            MethodType(partial(replacement,
                               getattr(original_class, replacement.__name__)),
                       None,
                       original_class))

 And the replacement becomes:

  def CustomizeBrowserOptions(original, self, options):
    original(self, options)
    power.PowerMetric.CustomizeBrowserOptions(options)
  _replace(CustomizeBrowserOptions)


 If we want to go a step further, we can pass a class to the decorator, and it
will use the 
bound method of the class to find which method to decorate, and the will be no
more boilerplate than in the MixIn case.

[dtu, 2014-04-14 18:47:44]

On 2014/04/11 06:57:31, qsr wrote:
> On 2014/04/10 19:52:43, dtu wrote:
> > On 2014/04/10 07:39:46, qsr wrote:
> > > On 2014/04/09 18:04:35, dtu wrote:
> > > > On 2014/04/09 12:26:05, qsr wrote:
> > > > > Oh yes also, not ready to review, just want an answer on the idea,
will
> > > clean
> > > > it
> > > > > up, and use it on all measurement if we agree...
> > > > 
> > > > Are you familiar with MixIns? That would be a cleaner way of doing the
> same
> > > > thing.
> > > > (A MixIn is just using multiple inheritance to override/wrap the class's
> > > > methods.)
> > > 
> > >  I didn't know python had mixin... And I must be missing something when
> > reading
> > > the doc I do not see how I can do what I want with it.
> > > 
> > >  If I make my measurement extends my mixin, I will have to call super() on
> > every
> > > method for this to work, while by using my decorator, I can have the
metric
> > > decorator choose with method it overrides and be responsible for then
> calling
> > > the original method. Does it make sense or did I miss something obvious?
> > 
> > Ah, you're right, there's a tradeoff I didn't realize.
> 
>  There is another, and one that I need, I need to be able to decorate the
method
> before and after the call, which I do not see how to do with mixins.
> 
> > Using MixIns also reduces the boilerplate in the Decorator/MixIn itself.
> > 
> > Instead of:
> >   originalCustomizeBrowserOptions = original_class.CustomizeBrowserOptions
> >   def newCustomizeBrowserOptions(self, options):
> >     originalCustomizeBrowserOptions(self, options)
> >     power.PowerMetric.CustomizeBrowserOptions(options)
> >   original_class.CustomizeBrowserOptions = newCustomizeBrowserOptions
> > You have:
> >   def CustomizeBrowserOptions(self, options):
> >     super(PowerMetricMixIn, self).CustomizeBrowserOptions(options)
> >     power.PowerMetric.CustomizeBrowserOptions(options)
> > 
> > The tradeoff is that yes, if the measurement overrides methods, you'll need
to
> > call super. But a lot of measurements (e.g. smoothness.py) won't need to
> define
> > any methods.
> 
>  Well, a little helper function can do a lot for the boiler plate:
> 
>  def _replace(replacement):
>     """Replace the the function of original_class whith the same name as
>     replacement by replacement.
>     """
>     setattr(original_class,
>             replacement.__name__,
>             MethodType(partial(replacement,
>                                getattr(original_class, replacement.__name__)),
>                        None,
>                        original_class))
> 
>  And the replacement becomes:
> 
>   def CustomizeBrowserOptions(original, self, options):
>     original(self, options)
>     power.PowerMetric.CustomizeBrowserOptions(options)
>   _replace(CustomizeBrowserOptions)
> 
> 
>  If we want to go a step further, we can pass a class to the decorator, and it
> will use the 
> bound method of the class to find which method to decorate, and the will be no
> more boilerplate than in the MixIn case.

so abstraction

sgtm

[tonyg, 2014-11-14 20:36:30]

tonyg@chromium.org changed reviewers:
- tonyg@chromium.org