# HG changeset patch # User Gregory Szorc # Date 1514398112 25200 # Node ID 12a46ad67a3ca2f1ac9afe91956fc521df220d8c # Parent 87918218da70cac7ec380335aa6ff1334c10b518 smartset: split generatorset classes to avoid cycle I uncovered a cycle manifesting in a memory leak by running `hgperfrevset '::tip'`. The cycle was due to generatorset.__init__ assigning a bound method to self.__contains__. Internet sleuthing revealed that assigning a bound method to an instance attribute always creates a cycle. This commit creates two new variants of generatorset for the special cases of ascending and descending generators. The special implementations of __contains__ have been extracted to these classes where they are defined as __contains__. generatorset now implements __new__ and changes the spawned type to one of the new classes if needed. Differential Revision: https://phab.mercurial-scm.org/D1780 diff -r 87918218da70 -r 12a46ad67a3c mercurial/smartset.py --- a/mercurial/smartset.py Wed Dec 27 13:53:21 2017 -0600 +++ b/mercurial/smartset.py Wed Dec 27 11:08:32 2017 -0700 @@ -772,6 +772,16 @@ >>> xs.last() # cached 4 """ + def __new__(cls, gen, iterasc=None): + if iterasc is None: + typ = cls + elif iterasc: + typ = _generatorsetasc + else: + typ = _generatorsetdesc + + return super(generatorset, cls).__new__(typ) + def __init__(self, gen, iterasc=None): """ gen: a generator producing the values for the generatorset. @@ -782,13 +792,6 @@ self._genlist = [] self._finished = False self._ascending = True - if iterasc is not None: - if iterasc: - self.fastasc = self._iterator - self.__contains__ = self._asccontains - else: - self.fastdesc = self._iterator - self.__contains__ = self._desccontains def __nonzero__(self): # Do not use 'for r in self' because it will enforce the iteration @@ -814,36 +817,6 @@ self._cache[x] = False return False - def _asccontains(self, x): - """version of contains optimised for ascending generator""" - if x in self._cache: - return self._cache[x] - - # Use new values only, as existing values would be cached. - for l in self._consumegen(): - if l == x: - return True - if l > x: - break - - self._cache[x] = False - return False - - def _desccontains(self, x): - """version of contains optimised for descending generator""" - if x in self._cache: - return self._cache[x] - - # Use new values only, as existing values would be cached. - for l in self._consumegen(): - if l == x: - return True - if l < x: - break - - self._cache[x] = False - return False - def __iter__(self): if self._ascending: it = self.fastasc @@ -947,7 +920,45 @@ def __repr__(self): d = {False: '-', True: '+'}[self._ascending] - return '<%s%s>' % (type(self).__name__, d) + return '<%s%s>' % (type(self).__name__.lstrip('_'), d) + +class _generatorsetasc(generatorset): + """Special case of generatorset optimized for ascending generators.""" + + fastasc = generatorset._iterator + + def __contains__(self, x): + if x in self._cache: + return self._cache[x] + + # Use new values only, as existing values would be cached. + for l in self._consumegen(): + if l == x: + return True + if l > x: + break + + self._cache[x] = False + return False + +class _generatorsetdesc(generatorset): + """Special case of generatorset optimized for descending generators.""" + + fastdesc = generatorset._iterator + + def __contains__(self, x): + if x in self._cache: + return self._cache[x] + + # Use new values only, as existing values would be cached. + for l in self._consumegen(): + if l == x: + return True + if l < x: + break + + self._cache[x] = False + return False def spanset(repo, start=0, end=None): """Create a spanset that represents a range of repository revisions diff -r 87918218da70 -r 12a46ad67a3c tests/test-revset.t --- a/tests/test-revset.t Wed Dec 27 13:53:21 2017 -0600 +++ b/tests/test-revset.t Wed Dec 27 11:08:32 2017 -0700 @@ -1484,7 +1484,7 @@ $ hg debugrevspec -s 'last(0::)' * set: > + > 9 $ hg identify -r '0::' --num 9 diff -r 87918218da70 -r 12a46ad67a3c tests/test-revset2.t --- a/tests/test-revset2.t Wed Dec 27 13:53:21 2017 -0600 +++ b/tests/test-revset2.t Wed Dec 27 11:08:32 2017 -0700 @@ -152,7 +152,7 @@ * set: , - > + > 5 3 1 @@ -174,7 +174,7 @@ (symbol '5')))))) * set: , + , > 0 1 @@ -927,7 +927,7 @@ (symbol 'merge') None)) * set: - + 6 7