| Submitter | Gregory Szorc |
|---|---|
| Date | Sept. 8, 2014, 10 p.m. |
| Message ID | <b12f25dad733d8842472.1410213652@gps-mbp.local> |
| Download | mbox | patch |
| Permalink | /patch/5735/ |
| State | Deferred |
| Headers | show |
Comments
I isolated the performance regression down to this usage of _diffset. I suspect, the baseset optimization is likely pulling its weight here. We should probably move that into _diffset. Also, this patch series is really for observation only. The patches have no obvious positive changes (aside from revset #24 speed-up in this patch). I'm not sure if this is due to lack of test coverage or just the patches not being appropriate. Given how often I seem to be stumbling across slowdowns in revset performance, I'm inclined to believe the revsetbenchmarks.txt coverage is at least partially lacking. I encourage others to add revset "death tests" to improve matters. On 9/8/14 3:00 PM, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1410200439 25200 > # Mon Sep 08 11:20:39 2014 -0700 > # Node ID b12f25dad733d8842472bfb64f8fa80347cad2f0 > # Parent 3a75a1f6937b15a6edc5e7a5c838c8d4cdf9dc73 > revset: use _diffset in spanset.__sub__ > > This patch results in the following statistically significant changes in > revset benchmarks for a clone of the Firefox repository. > > revset #8: author(mpm) > wall 11.409557 comb 11.410000 user 11.360000 sys 0.050000 (best of 3) > wall 12.045913 comb 12.040000 user 12.000000 sys 0.040000 (best of 3) > > revset #9: author(lmoscovicz) or author(mpm) > wall 30.106085 comb 30.100000 user 29.980000 sys 0.120000 (best of 3) > wall 36.646229 comb 36.640000 user 36.490000 sys 0.150000 (best of 3) > > revset #10: author(mpm) or author(lmoscovicz) > wall 29.797047 comb 29.800000 user 29.690000 sys 0.110000 (best of 3) > wall 35.162670 comb 35.170000 user 35.010000 sys 0.160000 (best of 3) > > revset #22: max(::(tip~20) - obsolete()) > wall 0.966111 comb 0.970000 user 0.960000 sys 0.010000 (best of 10) > wall 1.167777 comb 1.170000 user 1.160000 sys 0.010000 (best of 8) > > revset #24: (not public() - obsolete()) > wall 0.365095 comb 0.360000 user 0.360000 sys 0.000000 (best of 25) > wall 0.219435 comb 0.220000 user 0.220000 sys 0.000000 (best of 42) > > Since all but one of these is a regression, there are merits for > rejecting this patch. > > diff --git a/mercurial/revset.py b/mercurial/revset.py > --- a/mercurial/revset.py > +++ b/mercurial/revset.py > @@ -2873,14 +2873,14 @@ class spanset(_orderedsetmixin): > else: > return orderedlazyset(self, x.__contains__, ascending=False) > > def __sub__(self, x): > - if isinstance(x, baseset): > - x = x.set() > - if self._start <= self._end: > - return orderedlazyset(self, lambda r: r not in x) > - else: > - return orderedlazyset(self, lambda r: r not in x, ascending=False) > + kwargs = {} > + if self.isascending() and x.isascending(): > + kwargs['ascending'] = True > + if self.isdescending() and x.isdescending(): > + kwargs['ascending'] = False > + return _diffset(self, x, **kwargs) > > def __add__(self, x): > kwargs = {} > if self.isascending() and x.isascending(): >
Patch
diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -2873,14 +2873,14 @@ class spanset(_orderedsetmixin): else: return orderedlazyset(self, x.__contains__, ascending=False) def __sub__(self, x): - if isinstance(x, baseset): - x = x.set() - if self._start <= self._end: - return orderedlazyset(self, lambda r: r not in x) - else: - return orderedlazyset(self, lambda r: r not in x, ascending=False) + kwargs = {} + if self.isascending() and x.isascending(): + kwargs['ascending'] = True + if self.isdescending() and x.isdescending(): + kwargs['ascending'] = False + return _diffset(self, x, **kwargs) def __add__(self, x): kwargs = {} if self.isascending() and x.isascending():