| Submitter | Pierre-Yves David |
|---|---|
| Date | Sept. 19, 2014, 10:33 p.m. |
| Message ID | <bfaab5271eb80fd28278.1411166037@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/5888/ |
| State | Accepted |
| Commit | 00c8abe64cf3dba4693db99f1c915f1692d5b95a |
| Headers | show |
Comments
On Fri, Sep 19, 2014 at 03:33:57PM -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1411080765 25200 > # Thu Sep 18 15:52:45 2014 -0700 > # Node ID bfaab5271eb80fd282784864193e0d57a1ceef89 > # Parent a2b305e889547516d7952ee8ecd67948cd8ff360 > revset: prefetch an attribute in _generatorset.__iter__ > > Python's attribute lookup are expensible, lets do less of them. > > This gives us a 7% speedup on this revset iteration (from 0.063403 to 0.059032) > > diff --git a/mercurial/revset.py b/mercurial/revset.py > --- a/mercurial/revset.py > +++ b/mercurial/revset.py > @@ -2655,16 +2655,17 @@ class _generatorset(object): > # > # Getting ride of it would provide an about 15% speed up on this > # iteration. > i = 0 > genlist = self._genlist > - consume = self._consumegen() > + nextrev = self._consumegen().next > + _len = len # cache global lookup Did this really make a difference? The caching of the __builtin__, I mean (caching nextrev is obviously going to save some time.) > while True: > - if i < len(genlist): > + if i < _len(genlist): > yield genlist[i] > else: > - yield consume.next() > + yield nextrev() > i += 1 > > def _consumegen(self): > cache = self._cache > genlist = self._genlist.append > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 09/24/2014 08:28 AM, Augie Fackler wrote: > On Fri, Sep 19, 2014 at 03:33:57PM -0700, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1411080765 25200 >> # Thu Sep 18 15:52:45 2014 -0700 >> # Node ID bfaab5271eb80fd282784864193e0d57a1ceef89 >> # Parent a2b305e889547516d7952ee8ecd67948cd8ff360 >> revset: prefetch an attribute in _generatorset.__iter__ >> >> Python's attribute lookup are expensible, lets do less of them. >> >> This gives us a 7% speedup on this revset iteration (from 0.063403 to 0.059032) >> >> diff --git a/mercurial/revset.py b/mercurial/revset.py >> --- a/mercurial/revset.py >> +++ b/mercurial/revset.py >> @@ -2655,16 +2655,17 @@ class _generatorset(object): >> # >> # Getting ride of it would provide an about 15% speed up on this >> # iteration. >> i = 0 >> genlist = self._genlist >> - consume = self._consumegen() >> + nextrev = self._consumegen().next >> + _len = len # cache global lookup > > Did this really make a difference? The caching of the __builtin__, I > mean (caching nextrev is obviously going to save some time.) Yes, the global is catch using LOAD_GLOBAL (or something like that) that does a dictionnary lookup on all context abov this function. caching the builtin in a local free variable makes it as expensive as loading a local variable (lookup in an array) I do not remember the interdependent speed boost but there was some.
On Wed, Sep 24, 2014 at 12:33 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > On 09/24/2014 08:28 AM, Augie Fackler wrote: >> >> On Fri, Sep 19, 2014 at 03:33:57PM -0700, Pierre-Yves David wrote: >>> >>> # HG changeset patch >>> # User Pierre-Yves David <pierre-yves.david@fb.com> >>> # Date 1411080765 25200 >>> # Thu Sep 18 15:52:45 2014 -0700 >>> # Node ID bfaab5271eb80fd282784864193e0d57a1ceef89 >>> # Parent a2b305e889547516d7952ee8ecd67948cd8ff360 >>> revset: prefetch an attribute in _generatorset.__iter__ >>> >>> Python's attribute lookup are expensible, lets do less of them. >>> >>> This gives us a 7% speedup on this revset iteration (from 0.063403 to >>> 0.059032) >>> >>> diff --git a/mercurial/revset.py b/mercurial/revset.py >>> --- a/mercurial/revset.py >>> +++ b/mercurial/revset.py >>> @@ -2655,16 +2655,17 @@ class _generatorset(object): >>> # >>> # Getting ride of it would provide an about 15% speed up on >>> this >>> # iteration. >>> i = 0 >>> genlist = self._genlist >>> - consume = self._consumegen() >>> + nextrev = self._consumegen().next >>> + _len = len # cache global lookup >> >> >> Did this really make a difference? The caching of the __builtin__, I >> mean (caching nextrev is obviously going to save some time.) > > > Yes, the global is catch using LOAD_GLOBAL (or something like that) that > does a dictionnary lookup on all context abov this function. caching the > builtin in a local free variable makes it as expensive as loading a local > variable (lookup in an array) Sigh. Closing the loop on this thread: I'm ready to push these, but am getting a rebased version from marmoute via irc and pulling from his wip server. > > I do not remember the interdependent speed boost but there was some. > > -- > Pierre-Yves David
Patch
diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -2655,16 +2655,17 @@ class _generatorset(object): # # Getting ride of it would provide an about 15% speed up on this # iteration. i = 0 genlist = self._genlist - consume = self._consumegen() + nextrev = self._consumegen().next + _len = len # cache global lookup while True: - if i < len(genlist): + if i < _len(genlist): yield genlist[i] else: - yield consume.next() + yield nextrev() i += 1 def _consumegen(self): cache = self._cache genlist = self._genlist.append