Patchwork [2,of,3] perfphases: add a flag to also include file access time

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1495499261 -7200
#      Tue May 23 02:27:41 2017 +0200
# Node ID c79b8f2a95a35ce1b41325d003337c65bc5bffca
# Parent  e7b7317363359a4bb15b5713ff034c6fd2008339
# EXP-Topic perf
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c79b8f2a95a3
perfphases: add a flag to also include file access time

The flag purges all phases data so we'll have to read the file from disk again.

On Tue, May 23, 2017 at 7:04 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1495499261 -7200
> #      Tue May 23 02:27:41 2017 +0200
> # Node ID c79b8f2a95a35ce1b41325d003337c65bc5bffca
> # Parent  e7b7317363359a4bb15b5713ff034c6fd2008339
> # EXP-Topic perf
> # Available At https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/ -r c79b8f2a95a3
> perfphases: add a flag to also include file access time
>
> The flag purges all phases data so we'll have to read the file from disk
> again.
>
> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -581,12 +581,25 @@ def perfpathcopies(ui, repo, rev1, rev2,
>      timer(d)
>      fm.end()
>
> -@command('perfphases', [], "")
> +def _clearphasecache(repo):
> +    unfi = repo.unfiltered()
> +    if '_phasecache' in vars(unfi):
> +        del repo._phasecache
> +        del repo._filecache['_phasecache']
>
>
I'm really not a fan of this pattern in this patch or the next one. We have
cache clearing APIs on repo instances: I think we should use those instead.
If we don't have a sufficient, minimally-scoped cache clearing API, I'd
rather invent a private method on localrepository than have code in perf.py
reaching into the bowels of caches.


> +@command('perfphases',
> +         [('', 'full', False, 'include file reading time too'),
> +         ], "")
>  def perfphases(ui, repo, **opts):
>      """benchmark phasesets computation"""
>      timer, fm = gettimer(ui, opts)
> -    phases = repo._phasecache
> +    _phases = repo._phasecache
> +    full = opts.get('full')
>      def d():
> +        phases = _phases
> +        if full:
> +            _clearphasecache(repo)
> +            phases = repo._phasecache
>          phases.invalidate()
>          phases.loadphaserevs(repo)
>      timer(d)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

On 05/26/2017 07:00 PM, Gregory Szorc wrote:
> On Tue, May 23, 2017 at 7:04 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1495499261 -7200
>     #      Tue May 23 02:27:41 2017 +0200
>     # Node ID c79b8f2a95a35ce1b41325d003337c65bc5bffca
>     # Parent  e7b7317363359a4bb15b5713ff034c6fd2008339
>     # EXP-Topic perf
>     # Available At
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>     #              hg pull
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/> -r
>     c79b8f2a95a3
>     perfphases: add a flag to also include file access time
>
>     The flag purges all phases data so we'll have to read the file from
>     disk again.
>
>     diff --git a/contrib/perf.py b/contrib/perf.py
>     --- a/contrib/perf.py
>     +++ b/contrib/perf.py
>     @@ -581,12 +581,25 @@ def perfpathcopies(ui, repo, rev1, rev2,
>          timer(d)
>          fm.end()
>
>     -@command('perfphases', [], "")
>     +def _clearphasecache(repo):
>     +    unfi = repo.unfiltered()
>     +    if '_phasecache' in vars(unfi):
>     +        del repo._phasecache
>     +        del repo._filecache['_phasecache']
>
>
> I'm really not a fan of this pattern in this patch or the next one. We
> have cache clearing APIs on repo instances: I think we should use those
> instead. If we don't have a sufficient, minimally-scoped cache clearing
> API, I'd rather invent a private method on localrepository than have
> code in perf.py reaching into the bowels of caches.

Perf is doing something very specific here. I do not think any other 
users will exists. So I'm a bit worried to complexify localrepo just for 
perf.py. I think limiting the hack to perf.py is better.

There are already existing similar cache invalidation in 
prefvolatilesset... but that was added by me last week o:-)

Cheers,