| Submitter | Durham Goode |
|---|---|
| Date | Feb. 3, 2015, 3:19 a.m. |
| Message ID | <83bf8c0ecb425fb64903.1422933599@dev2000.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/7620/ |
| State | Superseded |
| Commit | ed5e8a9598cea22b20503ef969cc796ad9a71ea9 |
| Headers | show |
Comments
On 02/03/2015 03:19 AM, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1422061563 28800 > # Fri Jan 23 17:06:03 2015 -0800 > # Node ID 83bf8c0ecb425fb64903d93918227c162bca111d > # Parent 8b88870cbd1eeefaee0af053ae36728f8c0a1847 > manifest: make lru size configurable > > On machines with lots of ram, it's beneficial to increase the lru size of the > manifest cache. On a large repo, configuring the lru to be size 10 can shave a > large rebase (~12 commits) down from 95s to 70s. The option for this appears to be 'ui.manifestcachesize' this is hardly a 'ui' option. We currently do not have a good section to fit this. (not really format, not really ui). maybe we could rename the `workers` section to `ressources` or `performances` or `tuning`. And put them both there. Patch code looks good otherwise (and feature is cool)
On Mon, Feb 02, 2015 at 07:19:59PM -0800, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1422061563 28800 > # Fri Jan 23 17:06:03 2015 -0800 > # Node ID 83bf8c0ecb425fb64903d93918227c162bca111d > # Parent 8b88870cbd1eeefaee0af053ae36728f8c0a1847 > manifest: make lru size configurable > > On machines with lots of ram, it's beneficial to increase the lru size of the > manifest cache. On a large repo, configuring the lru to be size 10 can shave a > large rebase (~12 commits) down from 95s to 70s. > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > --- a/mercurial/localrepo.py > +++ b/mercurial/localrepo.py > @@ -323,6 +323,9 @@ class localrepository(object): > maxchainlen = self.ui.configint('format', 'maxchainlen') > if maxchainlen is not None: > self.svfs.options['maxchainlen'] = maxchainlen > + manifestcachesize = self.ui.configint('ui', 'manifestcachesize') After looking at patch 4, I started channelling my inner marmoute, and manifestcachesize doesn't want to be in [ui] either. > + if manifestcachesize is not None: > + self.svfs.options['manifestcachesize'] = manifestcachesize > > def _writerequirements(self): > reqfile = self.vfs("requires", "w") > diff --git a/mercurial/manifest.py b/mercurial/manifest.py > --- a/mercurial/manifest.py > +++ b/mercurial/manifest.py > @@ -222,7 +222,11 @@ class manifest(revlog.revlog): > def __init__(self, opener): > # we expect to deal with not more than four revs at a time, This comment appears to be out of date. Are you sure it's worth making this a knob, rather than just doing some experimentation and changing the fixed value? > # during a commit --amend > - self._mancache = util.lrucachedict(4) > + cachesize = 4 > + opts = getattr(opener, 'options', None) > + if opts is not None: > + cachesize = opts.get('manifestcachesize', cachesize) > + self._mancache = util.lrucachedict(cachesize) > revlog.revlog.__init__(self, opener, "00manifest.i") > > def readdelta(self, node): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Tue, 2015-02-03 at 15:20 +0000, Pierre-Yves David wrote: > > On 02/03/2015 03:19 AM, Durham Goode wrote: > > # HG changeset patch > > # User Durham Goode <durham@fb.com> > > # Date 1422061563 28800 > > # Fri Jan 23 17:06:03 2015 -0800 > > # Node ID 83bf8c0ecb425fb64903d93918227c162bca111d > > # Parent 8b88870cbd1eeefaee0af053ae36728f8c0a1847 > > manifest: make lru size configurable > > > > On machines with lots of ram, it's beneficial to increase the lru size of the > > manifest cache. On a large repo, configuring the lru to be size 10 can shave a > > large rebase (~12 commits) down from 95s to 70s. > > The option for this appears to be > > 'ui.manifestcachesize' this is hardly a 'ui' option. We currently do not > have a good section to fit this. (not really format, not really ui). > maybe we could rename the `workers` section to `ressources` or > `performances` or `tuning`. And put them both there. We have precedent in format.chunkcachesize, so format seems the obvious place. You might be interested in investigating the performance impact of that variable, btw.
On 2/3/15 7:26 AM, Augie Fackler wrote: > On Mon, Feb 02, 2015 at 07:19:59PM -0800, Durham Goode wrote: >> # HG changeset patch >> # User Durham Goode <durham@fb.com> >> # Date 1422061563 28800 >> # Fri Jan 23 17:06:03 2015 -0800 >> # Node ID 83bf8c0ecb425fb64903d93918227c162bca111d >> # Parent 8b88870cbd1eeefaee0af053ae36728f8c0a1847 >> manifest: make lru size configurable >> >> On machines with lots of ram, it's beneficial to increase the lru size of the >> manifest cache. On a large repo, configuring the lru to be size 10 can shave a >> large rebase (~12 commits) down from 95s to 70s. >> >> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py >> --- a/mercurial/localrepo.py >> +++ b/mercurial/localrepo.py >> @@ -323,6 +323,9 @@ class localrepository(object): >> maxchainlen = self.ui.configint('format', 'maxchainlen') >> if maxchainlen is not None: >> self.svfs.options['maxchainlen'] = maxchainlen >> + manifestcachesize = self.ui.configint('ui', 'manifestcachesize') > After looking at patch 4, I started channelling my inner marmoute, and > manifestcachesize doesn't want to be in [ui] either. > >> + if manifestcachesize is not None: >> + self.svfs.options['manifestcachesize'] = manifestcachesize >> >> def _writerequirements(self): >> reqfile = self.vfs("requires", "w") >> diff --git a/mercurial/manifest.py b/mercurial/manifest.py >> --- a/mercurial/manifest.py >> +++ b/mercurial/manifest.py >> @@ -222,7 +222,11 @@ class manifest(revlog.revlog): >> def __init__(self, opener): >> # we expect to deal with not more than four revs at a time, > This comment appears to be out of date. Are you sure it's worth making > this a knob, rather than just doing some experimentation and changing > the fixed value? I think making this a knob is still good. Some of the servers we run hg on have a metric crap ton of RAM, and being able to crank this number up to some unreasonable value would be convenient. > >> # during a commit --amend >> - self._mancache = util.lrucachedict(4) >> + cachesize = 4 >> + opts = getattr(opener, 'options', None) >> + if opts is not None: >> + cachesize = opts.get('manifestcachesize', cachesize) >> + self._mancache = util.lrucachedict(cachesize) >> revlog.revlog.__init__(self, opener, "00manifest.i") >> >> def readdelta(self, node): >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> http://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -323,6 +323,9 @@ class localrepository(object): maxchainlen = self.ui.configint('format', 'maxchainlen') if maxchainlen is not None: self.svfs.options['maxchainlen'] = maxchainlen + manifestcachesize = self.ui.configint('ui', 'manifestcachesize') + if manifestcachesize is not None: + self.svfs.options['manifestcachesize'] = manifestcachesize def _writerequirements(self): reqfile = self.vfs("requires", "w") diff --git a/mercurial/manifest.py b/mercurial/manifest.py --- a/mercurial/manifest.py +++ b/mercurial/manifest.py @@ -222,7 +222,11 @@ class manifest(revlog.revlog): def __init__(self, opener): # we expect to deal with not more than four revs at a time, # during a commit --amend - self._mancache = util.lrucachedict(4) + cachesize = 4 + opts = getattr(opener, 'options', None) + if opts is not None: + cachesize = opts.get('manifestcachesize', cachesize) + self._mancache = util.lrucachedict(cachesize) revlog.revlog.__init__(self, opener, "00manifest.i") def readdelta(self, node):