[PATCH 1 of 8] perf: introduce safeattrsetter to replace direct attribute assignment

[FUJIWARA Katsunori]

At Mon, 8 Aug 2016 23:20:04 +0900,
Yuya Nishihara wrote:
> 
> On Mon, 08 Aug 2016 18:59:40 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1470649697 -32400
> > #      Mon Aug 08 18:48:17 2016 +0900
> > # Node ID e5f93a8eb06c9b189c4f2460a66fe8f2d7713655
> > # Parent  84a8de5ac901c00f4efec1ec2e0be24ebd970fd8
> > perf: introduce safeattrsetter to replace direct attribute assignment
> 
> > To avoid examining existence of attribute at each repetition, this
> > patch makes safeattrsetter() return the function, which actually
> > assigns a value to an attribute.
> 
> [snip]
> 
> > +def safeattrsetter(obj, name, ignoremissing=False):
> > +    """Ensure that 'obj' has 'name' attribute before execution of setattr
> > +
> > +    This function is aborted, if 'obj' doesn't have 'name' attribute
> > +    at runtime. This avoids overlooking removal of an attribute, which
> > +    breaks assumption of performance measurement, in the future.
> > +
> > +    This function returns the function to assign a value to 'name'
> > +    attribute. This "setter" function returns the function to restore
> > +    'name' attribute to old one.
> > +
> > +    If 'ignoremissing' is true, missing 'name' attribute doesn't cause
> > +    abortion, and this function returns None. This is useful to
> > +    examine an attribute, which isn't ensured in all Mercurial
> > +    versions.
> > +    """
> > +    if not util.safehasattr(obj, name):
> > +        if ignoremissing:
> > +            return None
> > +        raise error.Abort(("missing attribute %s of %s might break assumption"
> > +                           " of performance measurement") % (name, obj))
> > +
> > +    def setter(newvalue):
> > +        oldvalue = getattr(obj, name)
> 
> If you care for the cost of safehasattr() (that could load a propertycache?),
> getattr() should be removed from the perf loop as well.
> 
> > +        setattr(obj, name, newvalue)
> > +        def restorer():
> > +            setattr(obj, name, oldvalue)
> > +        return restorer
> 
> Maybe this could be a class to avoid long closure chain.
> 
>   s = safeattrsetter(obj, name)  # remember current obj.name (=v0)
>   s.set(v1)
>   s.set(v2)
>   s.restore()  # restore v0, not v1

That looks better. I'll post revised one, thank you.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp