( )⚙ D4120 shortest: use nodetree for finding shortest node within revset

This is an archive of the discontinued Mercurial Phabricator instance.

shortest: use nodetree for finding shortest node within revset
ClosedPublic

Authored by martinvonz on Aug 5 2018, 3:58 AM.

Details

Summary

This speeds up hg log -T '{shortest(node,1)}\n' in my repo from 12s
to 4.5s. That's very close to the 4.1s it takes without the
disambiguation revset configured. My repo has 69.5k revisions, of
which 550 were in the configured revset ("not public()").

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Aug 5 2018, 3:58 AM
martinvonz updated this revision to Diff 9926.Aug 5 2018, 4:03 AM
martinvonz updated this revision to Diff 9985.Aug 6 2018, 9:24 AM
martinvonz updated this revision to Diff 10044.Aug 7 2018, 2:27 AM
martinvonz updated this revision to Diff 10084.Aug 8 2018, 8:09 PM
martinvonz updated this revision to Diff 10095.Aug 9 2018, 3:27 AM
martinvonz updated this revision to Diff 10458.Aug 20 2018, 3:35 AM

I have rebased this series and run all tests in pure and native mode, so this should be ready to queue as far as I can tell.

yuja added a subscriber: yuja.Aug 20 2018, 6:51 PM

test-revisions.t fails probably because null. I've queued up to the previous
patch.

static int nt_init_py(nodetree *self, PyObject *args)
{

  • PyObject *index;

+ indexObject *index;

Dropped this change as it's unrelated to this patch, and the index variable
is casted to indexObject* anyway.

	unsigned capacity;
  • if (!PyArg_ParseTuple(args, "O!I", &index, &capacity))

+ if (!PyArg_ParseTuple(args, "O!I", &indexType, &index, &capacity))

		return -1;

Moved this to D4118.

	return nt_init(self, (indexObject*)index, capacity);
In D4120#66662, @yuja wrote:

test-revisions.t fails probably because null.

Hmm, it passes for me. How does it fail for you?

I've queued up to the previous
patch.

static int nt_init_py(nodetree *self, PyObject *args)
{

  • PyObject *index;

+ indexObject *index;

Dropped this change as it's unrelated to this patch, and the index variable
is casted to indexObject* anyway.

	unsigned capacity;
  • if (!PyArg_ParseTuple(args, "O!I", &index, &capacity))

+ if (!PyArg_ParseTuple(args, "O!I", &indexType, &index, &capacity))

		return -1;

Moved this to D4118.

	return nt_init(self, (indexObject*)index, capacity);

Thanks!

martinvonz updated this revision to Diff 10485.Aug 20 2018, 7:02 PM
yuja added a comment.Aug 21 2018, 7:42 AM
Hmm, it passes for me. How does it fail for you?

Okay, it's a real bug. Filling Py_ssize_t as int.

static PyObject *nt_insert_py(nodetree *self, PyObject *args)
{
	Py_ssize_t rev;
	const char *node;
	if (!PyArg_ParseTuple(args, "i", &rev))
Traceback (most recent call last):
  File "hg", line 41, in <module>
    dispatch.run()
  File "mercurial/dispatch.py", line 90, in run
    status = dispatch(req)
  File "mercurial/dispatch.py", line 213, in dispatch
    ret = _runcatch(req) or 0
  File "mercurial/dispatch.py", line 354, in _runcatch
    return _callcatch(ui, _runcatchfunc)
  File "mercurial/dispatch.py", line 362, in _callcatch
    return scmutil.callcatch(ui, func)
  File "mercurial/scmutil.py", line 164, in callcatch
    return func()
  File "mercurial/dispatch.py", line 344, in _runcatchfunc
    return _dispatch(req)
  File "mercurial/dispatch.py", line 982, in _dispatch
    cmdpats, cmdoptions)
  File "mercurial/dispatch.py", line 728, in runcommand
    ret = _runcommand(ui, options, cmd, d)
  File "mercurial/dispatch.py", line 990, in _runcommand
    return cmdfunc()
  File "mercurial/dispatch.py", line 979, in <lambda>
    d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
  File "mercurial/util.py", line 1531, in check
    return func(*args, **kwargs)
  File "mercurial/dispatch.py", line 592, in __call__
    return util.checksignature(self.fn)(ui, *args, **opts)
  File "mercurial/util.py", line 1531, in check
    return func(*args, **kwargs)
  File "mercurial/commands.py", line 3643, in log
    displayfn(ui, repo, revs, displayer, getrenamed)
  File "mercurial/logcmdutil.py", line 897, in displayrevs
    displayer.show(ctx, copies=copies)
  File "mercurial/logcmdutil.py", line 186, in show
    self._show(ctx, copies, props)
  File "mercurial/logcmdutil.py", line 460, in _show
    self.ui.write(self.t.render(key, props))
  File "mercurial/templater.py", line 926, in render
    return b''.join(self.generate(t, mapping))
  File "mercurial/util.py", line 1442, in increasingchunks
    for chunk in source:
  File "mercurial/templateutil.py", line 659, in flatten
    for i in thing:
  File "mercurial/templateutil.py", line 853, in runtemplate
    yield evalrawexp(context, mapping, arg)
  File "mercurial/templateutil.py", line 707, in evalrawexp
    return func(context, mapping, data)
  File "mercurial/templatefuncs.py", line 634, in shortest
    return scmutil.shortesthexnodeidprefix(repo, node, minlength, cache)
  File "mercurial/scmutil.py", line 531, in shortesthexnodeidprefix
    nodetree.insert(r)
ValueError: revlog index out of range
yuja added a comment.Aug 21 2018, 7:45 AM

And can you bump cext version again as new code depends on nodetree.insert()?

In D4120#66682, @yuja wrote:
Hmm, it passes for me. How does it fail for you?

Okay, it's a real bug. Filling Py_ssize_t as int.

Fixed that and bumped the version. Thanks for helping me find that.

This revision was automatically updated to reflect the committed changes.