[PATCH 6 of 6] revset: fix a crash with 'roots(wdir())'

Yuya Nishihara yuya at tcha.org
Mon Jul 20 06:46:41 CDT 2015


On Sat, 18 Jul 2015 17:16:14 +0200, Pierre-Yves David wrote:
> On 07/04/2015 03:29 PM, Yuya Nishihara wrote:
> > I think (1) would be the hard way. My current idea is similar to (2), but
> > raising exception instead of devel-warning which needs 'ui' object.
> >
> >    cl.node(wdirrev) -> wdirid
> >    cl.rev(wdirnode) -> wdirrev
> >    cl.parentrevs(wdirrev) -> UnsupportedNodeError
> 
> The goal here would be to avoid to crash (crash to the user are bad) in
> multiple place where people will be using old code (especially
> extensions). Raising an exception would not help in this regard (except
> it would be a dedicated error)

Perhaps it can be caught at dispatch().

We might want to translate it to devel-warning at revset.getset(), but I'm
unsure if it go well because revsets are evaluated lazily.

> But as you pointed, cl have neither a repo or a ui, which limit our
> options a lot,
> 
> > Then, cl.parentrevs(r) will be able to fall back to repo[r].parents():
> >
> >    try:
> >        ps.add(cl.parentrevs(r)[0])
> >    except error.UnsupportedNodeError:
> >        ps.add(repo[r].p1().rev())
> >
> > The overhead will be minimal because 'wdir()' will be rarely used.
> 
> Setting up the exception catching will likely have an overhead:
> 
> no-except: 1.03 ms
> 
>    for i in xrange(2**16)
>      i
> 
> with-except: 1.57 ms
> 
>    for i in xrange(2**16)
>      try:
>        i
>      except:
>        pass

Yes, there are two extra opcodes in hot loop.

  (without try-except)

        >>   13 FOR_ITER                10 (to 26)
             16 STORE_FAST               0 (i)
  3          19 LOAD_FAST                0 (i)
             22 POP_TOP
             23 JUMP_ABSOLUTE           13

  (with try-except)

        >>   13 FOR_ITER                24 (to 40)
             16 STORE_FAST               0 (i)
  3          19 SETUP_EXCEPT             8 (to 30)
  4          22 LOAD_FAST                0 (i)
             25 POP_TOP
             26 POP_BLOCK
             27 JUMP_ABSOLUTE           13

If we can't accept this overhead, we'll probably have to move try-except
out of the loop:

  try:
      for r in ...:
          ps.add(cl.parentrevs(r)[0])
  except error.UnsupportedNodeError:
      # pay 2x penalty if wdir exists
      for r in ...:
          try:
              ps.add(cl.parentrevs(r)[0])
          except error.UnsupportedNodeError:
              ps.add(repo[r].p1().rev())


More information about the Mercurial-devel mailing list