Patchwork [2,of,2] revsetbenchmark: do not abort on failure to run a revset

login
register
mail settings
Submitter Pierre-Yves David
Date June 21, 2015, 3:38 a.m.
Message ID <da62bc192d6302180309.1434857923@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/9738/
State Accepted
Commit ae22d3048075d0a4e4daece4bdb231a1d939f97e
Delegated to: Augie Fackler
Headers show

Comments

Pierre-Yves David - June 21, 2015, 3:38 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1434842530 25200
#      Sat Jun 20 16:22:10 2015 -0700
# Node ID da62bc192d63021803097a267d48e66f9f64e9fe
# Parent  75305531b0c92e31b35a3357774597ac78b1a1fd
revsetbenchmark: do not abort on failure to run a revset

Instead of aborting the whole process, we just skip entry for revset that
failed to run.
Augie Fackler - June 22, 2015, 2:23 p.m.
On Sat, Jun 20, 2015 at 08:38:43PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1434842530 25200
> #      Sat Jun 20 16:22:10 2015 -0700
> # Node ID da62bc192d63021803097a267d48e66f9f64e9fe
> # Parent  75305531b0c92e31b35a3357774597ac78b1a1fd
> revsetbenchmark: do not abort on failure to run a revset
>
> Instead of aborting the whole process, we just skip entry for revset that
> failed to run.

I'm not super-thrilled with this. Could we be capturing the error
output somehow?

>
> diff --git a/contrib/revsetbenchmarks.py b/contrib/revsetbenchmarks.py
> --- a/contrib/revsetbenchmarks.py
> +++ b/contrib/revsetbenchmarks.py
> @@ -60,11 +60,11 @@ def perf(revset, target=None):
>          print >> sys.stderr, 'abort: cannot run revset benchmark: %s' % exc.cmd
>          if exc.output is None:
>              print >> sys.stderr, '(no ouput)'
>          else:
>              print >> sys.stderr, exc.output
> -        sys.exit(exc.returncode)
> +        return None
>
>  outputre = re.compile(r'! wall (\d+.\d+) comb (\d+.\d+) user (\d+.\d+) '
>                        'sys (\d+.\d+) \(best of (\d+)\)')
>
>  def parseoutput(output):
> @@ -158,12 +158,17 @@ def formattiming(value):
>
>  _marker = object()
>  def printresult(variants, idx, data, maxidx, verbose=False, reference=_marker):
>      """print a line of result to stdout"""
>      mask = '%%0%ii) %%s' % idxwidth(maxidx)
> +
>      out = []
>      for var in variants:
> +        if data[var] is None:
> +            out.append('error   ')
> +            out.append(' ' * 4)
> +            continue
>          out.append(formattiming(data[var]['wall']))
>          if reference is not _marker:
>              factor = None
>              if reference is not None:
>                  factor = getfactor(reference[var], data[var], 'wall')
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - June 22, 2015, 5:12 p.m.
On 06/22/2015 07:23 AM, Augie Fackler wrote:
> On Sat, Jun 20, 2015 at 08:38:43PM -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1434842530 25200
>> #      Sat Jun 20 16:22:10 2015 -0700
>> # Node ID da62bc192d63021803097a267d48e66f9f64e9fe
>> # Parent  75305531b0c92e31b35a3357774597ac78b1a1fd
>> revsetbenchmark: do not abort on failure to run a revset
>>
>> Instead of aborting the whole process, we just skip entry for revset that
>> failed to run.
>
> I'm not super-thrilled with this. Could we be capturing the error
> output somehow?

Meh, the error output is alreay forwarded to the stderr when we try to 
run the revsets, and this is "good enough" for now. We will not reused 
the error output in the result summary because it is will never fit in 
any column.

As we currently have no user to capture it, we don't. It is simple 
enough to capture it when we need it later.
Augie Fackler - June 22, 2015, 10:43 p.m.
On Mon, Jun 22, 2015 at 10:12:46AM -0700, Pierre-Yves David wrote:
>
>
> On 06/22/2015 07:23 AM, Augie Fackler wrote:
> >On Sat, Jun 20, 2015 at 08:38:43PM -0700, Pierre-Yves David wrote:
> >># HG changeset patch
> >># User Pierre-Yves David <pierre-yves.david@fb.com>
> >># Date 1434842530 25200
> >>#      Sat Jun 20 16:22:10 2015 -0700
> >># Node ID da62bc192d63021803097a267d48e66f9f64e9fe
> >># Parent  75305531b0c92e31b35a3357774597ac78b1a1fd
> >>revsetbenchmark: do not abort on failure to run a revset
> >>
> >>Instead of aborting the whole process, we just skip entry for revset that
> >>failed to run.
> >
> >I'm not super-thrilled with this. Could we be capturing the error
> >output somehow?
>
> Meh, the error output is alreay forwarded to the stderr when we try to run
> the revsets, and this is "good enough" for now. We will not reused the error
> output in the result summary because it is will never fit in any column.

Ah, okay, I didn't have that context. Queued with delight.

> As we currently have no user to capture it, we don't. It is simple enough to
> capture it when we need it later.
>
> --
> Pierre-Yves David

Patch

diff --git a/contrib/revsetbenchmarks.py b/contrib/revsetbenchmarks.py
--- a/contrib/revsetbenchmarks.py
+++ b/contrib/revsetbenchmarks.py
@@ -60,11 +60,11 @@  def perf(revset, target=None):
         print >> sys.stderr, 'abort: cannot run revset benchmark: %s' % exc.cmd
         if exc.output is None:
             print >> sys.stderr, '(no ouput)'
         else:
             print >> sys.stderr, exc.output
-        sys.exit(exc.returncode)
+        return None
 
 outputre = re.compile(r'! wall (\d+.\d+) comb (\d+.\d+) user (\d+.\d+) '
                       'sys (\d+.\d+) \(best of (\d+)\)')
 
 def parseoutput(output):
@@ -158,12 +158,17 @@  def formattiming(value):
 
 _marker = object()
 def printresult(variants, idx, data, maxidx, verbose=False, reference=_marker):
     """print a line of result to stdout"""
     mask = '%%0%ii) %%s' % idxwidth(maxidx)
+
     out = []
     for var in variants:
+        if data[var] is None:
+            out.append('error   ')
+            out.append(' ' * 4)
+            continue
         out.append(formattiming(data[var]['wall']))
         if reference is not _marker:
             factor = None
             if reference is not None:
                 factor = getfactor(reference[var], data[var], 'wall')