[PATCH memctx-cache] memctx: always use cache for filectxfn
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Mon Jun 12 00:06:37 UTC 2017
At Sun, 11 Jun 2017 15:20:25 -0700,
Sean Farley wrote:
>
> FUJIWARA Katsunori <foozy at lares.dti.ne.jp> writes:
>
> > At Sat, 10 Jun 2017 16:01:23 -0700,
> > Sean Farley wrote:
> >>
> >> # HG changeset patch
> >> # User Sean Farley <sean at farley.io>
> >> # Date 1497135618 25200
> >> # Sat Jun 10 16:00:18 2017 -0700
> >> # Branch memctx-cache
> >> # Node ID 12f2faa6019f497ba08960564cfceff80250e75a
> >> # Parent 7e9d0d8ff938dcf8ca193c17db5321a05a48e718
> >> memctx: always use cache for filectxfn
> >>
> >> I don't see a downside to doing this unless I'm missing something.
> >>
> >> diff --git a/mercurial/context.py b/mercurial/context.py
> >> index 7ce34af..348c13c 100644
> >> --- a/mercurial/context.py
> >> +++ b/mercurial/context.py
> >> @@ -2103,18 +2103,19 @@ class memctx(committablectx):
> >> self._files = files
> >> if branch is not None:
> >> self._extra['branch'] = encoding.fromlocal(branch)
> >> self.substate = {}
> >>
> >> + self._filectxfn = filectxfn
> >> if isinstance(filectxfn, patch.filestore):
> >> self._filectxfn = memfilefrompatch(filectxfn)
> >> elif not callable(filectxfn):
> >> # if store is not callable, wrap it in a function
> >> self._filectxfn = memfilefromctx(filectxfn)
> >> - else:
> >> - # memoizing increases performance for e.g. vcs convert scenarios.
> >> - self._filectxfn = makecachingfilectxfn(filectxfn)
> >> +
> >> + # memoizing increases performance for e.g. vcs convert scenarios.
> >> + self._filectxfn = makecachingfilectxfn(filectxfn)
> >
> > This change seems to cache also results of memfilefrompatch() and
> > makecachingfilectxfn(). If so, should makecachingfilectxfn() be
> > applied on self._filectxfn ? Or:
> >
> > ================
> > self.substate = {}
> >
> > if isinstance(filectxfn, patch.filestore):
> > - self._filectxfn = memfilefrompatch(filectxfn)
> > + filectxfn = memfilefrompatch(filectxfn)
> > elif not callable(filectxfn):
> > # if store is not callable, wrap it in a function
> > - self._filectxfn = memfilefromctx(filectxfn)
> > - else:
>
> The third type of object that can be sent is a function that takes
> (repo, memctx, path), so we can't get rid of the 'else:' like this.
With your change, wrapping original "filectxfn" by memfilefrompatch()
and memfilefromctx() is meaningless (and initial "self._filectxfn =
filectxfn" in your change, too), because previous "self._filectxfn" (=
wrapping result) is always overwritten by result of
makecachingfilectxfn() with original "filectxfn" argument.
"self._filectxfn = makecachingfilectxfn(self._filectxfn)" seems OK, IMHO.
> My
> main question in this patch is: can we cache everything from filectxfn?
> If not, that's fine (and I would personally document the reason in the
> code).
I think that results of memctx._filectxfn() can be always cached, too.
--
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list