[PATCH 1 of 2] perf: add perfbdiff

Mads Kiilerich mads at kiilerich.com
Sun Nov 6 11:15:24 EST 2016


On 11/06/2016 08:42 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1478414512 25200
> #      Sat Nov 05 23:41:52 2016 -0700
> # Node ID e65ef545a3dcb46fe0f1798e76ea17f9f9323452
> # Parent  f01367faa792635ad2f7a6b175ae3252292b5121
> perf: add perfbdiff
>
> bdiff shows up a lot in profiling. I think it would be useful to have
> a perf command that runs bdiff over and over so we can find hot spots.
>
> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -25,6 +25,7 @@ import random
>   import sys
>   import time
>   from mercurial import (
> +    bdiff,
>       changegroup,
>       cmdutil,
>       commands,
> @@ -746,6 +747,30 @@ def perffncacheencode(ui, repo, **opts):
>       timer(d)
>       fm.end()
>   
> + at command('perfbdiff', revlogopts + formatteropts, '-c|-m|FILE REV')

I would prefer to keep it simple and consistently use -r for specifying 
revisions.

> +def perfbdiff(ui, repo, file_, rev=None, **opts):
> +    """benchmark a bdiff between a revision and its delta parent"""
> +    if opts.get('changelog') or opts.get('manifest'):
> +        file_, rev = None, file_
> +    elif rev is None:
> +        raise error.CommandError('perfbdiff', 'invalid arguments')
> +
> +    r = cmdutil.openrevlog(repo, 'perfbdiff', file_, opts)
> +
> +    node = r.lookup(rev)
> +    rev = r.rev(node)

This might be a stupid lazy question that essential is a request for 
more clarity in code and docstring:
Why this back and forth between rev and node?
Must rev always be a filelog revision ... or is it a changelog revision 
which then is changed to revlog revision while reusing the variable 
name? Perhaps reduce confusion by using different names.

> +    dp = r.deltaparent(rev)

Should it also consider aggressivemergedeltas?

Or perhaps more generally: should it be possible to compare any two 
revisions - especially for filelogs, to also cover the use case of diff?


More important: patch 2 LGTM.

/Mads



More information about the Mercurial-devel mailing list