From c6741e4c3a6b1818432cdbb9959bc585bba40fe3 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Fri, 6 Nov 2020 15:29:18 -0800 Subject: [PATCH] Back out "smartset: back out use Rust reentrant generator for generatorset" Summary: The original issue was a rust-cpython bug, solved by D24698226, or https://github.com/dgrunwald/rust-cpython/pull/244. Original commit changeset: 08f598df0892 Reviewed By: sfilipco Differential Revision: D24759765 fbshipit-source-id: f9a1359cfce68c8754ddd1bcb8bfc54bf75af7ff --- eden/scm/edenscm/mercurial/scmutil.py | 8 ++ eden/scm/edenscm/mercurial/smartset.py | 118 ++++++++---------- .../bindings/modules/pyerror/src/lib.rs | 2 +- eden/scm/tests/test-inherit-mode.t | 4 +- 4 files changed, 65 insertions(+), 67 deletions(-) diff --git a/eden/scm/edenscm/mercurial/scmutil.py b/eden/scm/edenscm/mercurial/scmutil.py index fd9f74e56e..0c23a06c73 100644 --- a/eden/scm/edenscm/mercurial/scmutil.py +++ b/eden/scm/edenscm/mercurial/scmutil.py @@ -34,6 +34,7 @@ from . import ( pycompat, revsetlang, similar, + smartset, url, util, vfs, @@ -526,6 +527,10 @@ def revsingle(repo, revspec, default=".", localalias=None): if not revspec and revspec != 0: return repo[default] + # Used by amend/common calling rebase.rebase with non-string opts. + if isinstance(revspec, (int, type(1 << 63))): + return repo[revspec] + l = revrange(repo, [revspec], localalias=localalias) if not l: raise error.Abort(_("empty revision set")) @@ -599,6 +604,9 @@ def revrange(repo, specs, localalias=None): use repo.revs (ignores user-defined revset aliases) or repo.anyrevs (respects user-defined revset aliases) instead. """ + # Used by amend/common calling rebase.rebase with non-string opts. + if isinstance(specs, smartset.abstractsmartset): + return specs allspecs = [] for spec in specs: if isinstance(spec, int): diff --git a/eden/scm/edenscm/mercurial/smartset.py b/eden/scm/edenscm/mercurial/smartset.py index 0bf3d8985c..15603764e2 100644 --- a/eden/scm/edenscm/mercurial/smartset.py +++ b/eden/scm/edenscm/mercurial/smartset.py @@ -1249,11 +1249,11 @@ class generatorset(abstractsmartset): """ gen: a generator producing the values for the generatorset. """ - self._gen = gen + rgen = bindings.threading.RGenerator(gen) + self._rgen = rgen + self._containschecked = 0 self._asclist = None self._cache = {} - self._genlist = [] - self._finished = False self._ascending = True if iterasc is not None: if iterasc: @@ -1262,57 +1262,79 @@ class generatorset(abstractsmartset): else: self.fastdesc = self._iterator self.__contains__ = self._desccontains + self._iterasc = iterasc + + @property + def _finished(self): + return self._rgen.completed() def __nonzero__(self): # Do not use 'for r in self' because it will enforce the iteration # order (default ascending), possibly unrolling a whole descending # iterator. - if self._genlist: + if self._rgen.list(): return True - for r in self._consumegen(): + try: + next(self._rgen.iter()) return True - return False + except StopIteration: + return False __bool__ = __nonzero__ def __contains__(self, x): - if x in self._cache: - return self._cache[x] + cache = self._cache + if x in cache: + return cache[x] - # Use new values only, as existing values would be cached. - for l in self._consumegen(): + checked = self._containschecked + for l in self._rgen.iter(skip=checked): + checked += 1 + cache[l] = True if l == x: + self._containschecked = checked return True + self._containschecked = checked - self._cache[x] = False + 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] + cache = self._cache + if x in cache: + return cache[x] - # Use new values only, as existing values would be cached. - for l in self._consumegen(): + checked = self._containschecked + for l in self._rgen.iter(skip=checked): + checked += 1 + cache[l] = True if l == x: + self._containschecked = checked return True if l > x: break + self._containschecked = checked - self._cache[x] = False + 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] + cache = self._cache + if x in cache: + return cache[x] - # Use new values only, as existing values would be cached. - for l in self._consumegen(): + checked = self._containschecked + for l in self._rgen.iter(skip=checked): + checked += 1 + cache[l] = True if l == x: + self._containschecked = checked return True if l < x: break + self._containschecked = checked self._cache[x] = False return False @@ -1325,58 +1347,28 @@ class generatorset(abstractsmartset): if it is not None: return it() # we need to consume the iterator - for x in self._consumegen(): - pass + self._fulllist() # recall the same code return iter(self) def _iterator(self): if self._finished: - return iter(self._genlist) + return iter(self._rgen.list()) + return self._rgen.iter() - # We have to use this complex iteration strategy to allow multiple - # iterations at the same time. We need to be able to catch revision - # removed from _consumegen and added to genlist in another instance. - # - # Getting rid of it would provide an about 15% speed up on this - # iteration. - genlist = self._genlist - nextgen = self._consumegen() - _len, _next = len, next # cache global lookup - - def gen(): - i = 0 - while True: - if i < _len(genlist): - yield genlist[i] - else: - try: - yield _next(nextgen) - except StopIteration: - return - i += 1 - - return gen() - - def _consumegen(self): - cache = self._cache - genlist = self._genlist.append - for item in self._gen: - cache[item] = True - genlist(item) - yield item + def _fulllist(self): if not self._finished: - self._finished = True - asc = self._genlist[:] - asc.sort() + self._rgen.itertoend() + assert self._finished + if self._asclist is None: + asc = sorted(self._rgen.list()) self._asclist = asc self.fastasc = asc.__iter__ self.fastdesc = asc.__reversed__ + return self._rgen.list() def __len__(self): - for x in self._consumegen(): - pass - return len(self._genlist) + return len(self._fulllist()) def sort(self, reverse=False): self._ascending = not reverse @@ -1403,8 +1395,7 @@ class generatorset(abstractsmartset): it = self.fastdesc if it is None: # we need to consume all and try again - for x in self._consumegen(): - pass + self._fulllist() return self.first() return next(it(), None) @@ -1415,8 +1406,7 @@ class generatorset(abstractsmartset): it = self.fastasc if it is None: # we need to consume all and try again - for x in self._consumegen(): - pass + self._fulllist() return self.last() return next(it(), None) diff --git a/eden/scm/edenscmnative/bindings/modules/pyerror/src/lib.rs b/eden/scm/edenscmnative/bindings/modules/pyerror/src/lib.rs index ca12f15ee1..8963de8ab1 100644 --- a/eden/scm/edenscmnative/bindings/modules/pyerror/src/lib.rs +++ b/eden/scm/edenscmnative/bindings/modules/pyerror/src/lib.rs @@ -8,7 +8,7 @@ use cpython::*; use cpython_ext::{error, ResultPyErrExt}; -use taggederror::{intentional_bail, intentional_error, CommonMetadata, Fault, FilteredAnyhow}; +use taggederror::{intentional_bail, intentional_error, CommonMetadata, FilteredAnyhow}; use taggederror_util::AnyhowEdenExt; py_exception!(error, IndexedLogError); diff --git a/eden/scm/tests/test-inherit-mode.t b/eden/scm/tests/test-inherit-mode.t index 066e614734..d9b8e4a0f3 100644 --- a/eden/scm/tests/test-inherit-mode.t +++ b/eden/scm/tests/test-inherit-mode.t @@ -103,7 +103,7 @@ new directories are setgid 00664 ./.hg/store/metalog/roots/log 00664 ./.hg/store/metalog/roots/meta 00600 ./.hg/store/requires - 00600 ./.hg/store/tip + 006?? ./.hg/store/tip (glob) 00660 ./.hg/store/undo 00660 ./.hg/store/undo.backupfiles 00660 ./.hg/store/undo.bookmarks @@ -171,7 +171,7 @@ XXX: treestate and allheads do not really respect this rule 00664 ../push/.hg/store/metalog/roots/log 00664 ../push/.hg/store/metalog/roots/meta 00660 ../push/.hg/store/requires - 00600 ../push/.hg/store/tip + 006?? ../push/.hg/store/tip (glob) 00660 ../push/.hg/store/undo 00660 ../push/.hg/store/undo.backupfiles 00660 ../push/.hg/store/undo.bookmarks