[PATCH 2 of 3] store: eliminate reference cycle in fncachestore

Simon Heimberg simohe at besonet.ch
Tue Jul 14 07:09:36 CDT 2009


Am Montag, den 13.07.2009, 19:31 +0200 schrieb Adrian Buehlmann:
> On 13.07.2009 17:31, Simon Heimberg wrote:
> > Am Montag, den 13.07.2009, 17:06 +0200 schrieb Adrian Buehlmann:
> >> On 13.07.2009 16:15, Simon Heimberg wrote:
> >>> # HG changeset patch
> >>> # User Simon Heimberg <simohe at besonet.ch>
> >>> # Date 1247241244 -7200
> >>> # Node ID d0d02a480e6a65cac1c86f7e74445c3e71ed173a
> >>> # Parent  a9ab0e06c244d08c1308b843d8b3385e9051788e
> >>> store: eliminate reference cycle in fncachestore
> >>>
> >>> override method instead of passing a bound method
> > see here:
> >>> pass defversion in store because it is not possible to add a property to a
> >>> method
> > to here
> >>> diff -r a9ab0e06c244 -r d0d02a480e6a mercurial/localrepo.py
> >>> --- a/mercurial/localrepo.py	Don Jul 09 13:23:27 2009 +0200
> >>> +++ b/mercurial/localrepo.py	Fre Jul 10 17:54:04 2009 +0200
> >>> @@ -106,7 +106,7 @@
> >>>              p = os.environ['HG_PENDING']
> >>>              if p.startswith(self.root):
> >>>                  c.readpending('00changelog.i.a')
> >>> -        self.sopener.defversion = c.version
> >>> +        self.store.defversion = c.version
> >>>          return c
> >>>  
> >>>      @propertycache
> >>> diff -r a9ab0e06c244 -r d0d02a480e6a mercurial/revlog.py
> >>> --- a/mercurial/revlog.py	Don Jul 09 13:23:27 2009 +0200
> >>> +++ b/mercurial/revlog.py	Fre Jul 10 17:54:04 2009 +0200
> >>> @@ -430,11 +430,12 @@
> >>>          self.nodemap = {nullid: nullrev}
> >>>          self.index = []
> >>>  
> >>> -        v = REVLOG_DEFAULT_VERSION
> >>> -        if hasattr(opener, "defversion"):
> >>> -            v = opener.defversion
> >>> +        try:
> >>> +            v = opener.im_self.defversion
> >>>              if v & REVLOGNG:
> >>>                  v |= REVLOGNGINLINEDATA
> >>> +        except AttributeError:
> >>> +            v = REVLOG_DEFAULT_VERSION
> >>>  
> >>>          i = ''
> >>>          try:
> >> What does this have to do with the change in store.py?
> > 
> > It is not possible to add a property to a method. Because opener is a method of store now,
> defversion can not be there anymore. It moved in the store class. Because contrib/dumprevlog
> uses a lambda function as opener, opener does not always have im_self.
> Instead of "if hasattr(opener, 'im_self') and hasattr(opener.im_self, 'defversion'):"
> try/except is used.
> 
> Hmm.
> 
> So, if I understand this correctly, the change in store.py
> then depends on these changes to revlog.py and localrepo.py, but
> not the other way round (the changes to revlog.py and localrepo.py
> do not depend on the change to store.py).
> 
> If this is correct, then I suggest to split this patch into
> two.
> 
> First patch "move defversion property of opener into store" (or something,
> choose whatever text fits best), consisting of the changes to revlog.py and
> localrepo.py shown above.
> 
> Second patch "store: eliminate reference cycle in fncachestore"
> consisting of the change to store.py.
> 
> I think, this would make it easier to understand what you are
> trying to do here.
> 
> >>> diff -r a9ab0e06c244 -r d0d02a480e6a mercurial/store.py
> >>> --- a/mercurial/store.py	Don Jul 09 13:23:27 2009 +0200
> >>> +++ b/mercurial/store.py	Fre Jul 10 17:54:04 2009 +0200
> >>> @@ -171,9 +171,11 @@
> > [snip]
> > 

It can not be split because bought changes depend on the other.
* When localrepo.sopener is a (lambda) function, it does not have the
attribute im_self. This means (the instance of) store can not be
reached.
* When localrepo.sopener is the (bound) method store.opener, the
attribute defversion can not be added because methods can not have
attributes.

Have also a look to the patch influenced by your other feedback. It
needs less changes.


More information about the Mercurial-devel mailing list