GNU bug report logs - #17945
24.4.50; vc-git-annotate-command is too slow

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17945 in the body.
You can then email your comments to 17945 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: William Xu <william.xwl <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4.50; vc-git-annotate-command is too slow
Date: Sat, 5 Jul 2014 20:28:37 +0800

[Message part 1 (text/plain, inline)]

C-x v g on a git file is very slow, compare this:

    $ time git --no-pager  blame  -- vc-annotate.el 1>/dev/null

    real    0m1.432s
    user    0m1.364s
    sys    0m0.063s


    $ time git --no-pager  blame -C -C  -- vc-annotate.el 1>/dev/null

    real    0m23.477s
    user    0m22.058s
    sys    0m1.405s

It seems the -C -C options would slow down git hugely, making this
command almost useless.  Can we remove it from default option list?

-William

git version 1.9.3

In GNU Emacs 24.4.50.1 (i386-apple-darwin13.1.0, NS apple-appkit-1265.19)
 of 2014-04-16 on tokyolove.local
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --with-ns CC=clang'

Configured features:
ACL GNUTLS LIBXML2 ZLIB

Important settings:
  locale-coding-system: utf-8-unix

[Message part 2 (text/html, inline)]

Message #8 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry <dgutov <at> yandex.ru>
To: William Xu <william.xwl <at> gmail.com>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Sat, 05 Jul 2014 16:34:20 +0300

William Xu <william.xwl <at> gmail.com> writes:

> C-x v g on a git file is very slow, compare this:

What kind of repository are you doing this in?

For me, it always works quite fast (usually <1 s). 

> It seems the -C -C options would slow down git hugely, making this
> command almost useless.  Can we remove it from default option list?

These options provide a feature, so we shouldn't simply remove them.

Maybe we should provide a user option.

Message #11 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: William Xu <william.xwl <at> gmail.com>
To: Dmitry <dgutov <at> yandex.ru>
Cc: "17945 <at> debbugs.gnu.org" <17945 <at> debbugs.gnu.org>
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Sat, 5 Jul 2014 23:16:24 +0800

[Message part 1 (text/plain, inline)]

Dmitry <dgutov <at> yandex.ru>于2014年7月5日星期六写道：

> William Xu <william.xwl <at> gmail.com <javascript:;>> writes:
>
> > C-x v g on a git file is very slow, compare this:
>
> What kind of repository are you doing this in?
>
> For me, it always works quite fast (usually <1 s)


As you can see, I ran it on the emacs git mirror(git://
git.savannah.org/emacs.git).  I can see this slowness on my several git
repos where there are many files.  I often fall back on running
shell-command with 'git blame'.


>
> It seems the -C -C options would slow down git hugely, making this
> > command almost useless.  Can we remove it from default option list?
>
> These options provide a feature, so we shouldn't simply remove them.


There are many features, the question is whether this is good for default
setup.


-William




-- 
-William

[Message part 2 (text/html, inline)]

Message #14 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry <dgutov <at> yandex.ru>
Cc: William Xu <william.xwl <at> gmail.com>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Sat, 05 Jul 2014 12:15:40 -0400

>> It seems the -C -C options would slow down git hugely, making this
>> command almost useless.  Can we remove it from default option list?
> These options provide a feature, so we shouldn't simply remove them.

I think it makes sense to follow Git's defaults.

> Maybe we should provide a user option.

Indeed, vc-git-annotate-command should use (vc-switches 'git annotate).


        Stefan

Message #19 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 02:39:36 -0500

I find vc-annotate _unusably_ slow on the Emacs repo thanks to these -"C"s.
Eg for rmailsum.el: 4 sec versus 5.5 minutes.

Message #22 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 16:31:45 +0100

So what about this patch?

	Modified   lisp/vc/vc-git.el
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index a31c121..07ff083 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -120,6 +120,16 @@ (defcustom vc-git-diff-switches t
   :version "23.1"
   :group 'vc-git)
 
+(defcustom vc-git-annotate-switches t
+  "String or list of strings specifying switches for Git blame under VC.
+If nil, use the value of `vc-annotate-switches'.  If t, use no switches."
+  :type '(choice (const :tag "Unspecified" nil)
+		 (const :tag "None" t)
+		 (string :tag "Argument String")
+		 (repeat :tag "Argument List" :value ("") string))
+  :version "25.1"
+  :group 'vc-git)
+
 (defcustom vc-git-program "git"
   "Name of the Git executable (excluding any arguments)."
   :version "24.1"
@@ -1013,7 +1023,9 @@ (defun vc-git-revision-completion-table (files)
 
 (defun vc-git-annotate-command (file buf &optional rev)
   (let ((name (file-relative-name file)))
-    (vc-git-command buf 'async nil "blame" "--date=iso" "-C" "-C" rev "--" name)))
+    (apply #'vc-git-command buf 'async nil "blame" "--date=iso"
+	   (append (vc-switches 'git 'annotate)
+		   (list rev "--" name)))))
 
 (declare-function vc-annotate-convert-time "vc-annotate" (time))

Message #25 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Óscar Fuentes <ofv <at> wanadoo.es>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 18:12:35 +0200

> From: Óscar Fuentes <ofv <at> wanadoo.es>
> Date: Tue, 24 Feb 2015 16:31:45 +0100
> 
> So what about this patch?

Why not a prefix argument?  It's easier to set once when one needs it,
and is the usual way in Emacs to provide minor variations of commands.

Or maybe do both, for those who might want this optional behavior
always (although given the massive slow-down, and the general
dismissal of the issue among git users, I doubt that such a person
exists).

Thanks.

Message #28 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Óscar Fuentes
 <ofv <at> wanadoo.es>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 18:36:43 +0200

On 02/24/2015 06:12 PM, Eli Zaretskii wrote:

> Why not a prefix argument?  It's easier to set once when one needs it,
> and is the usual way in Emacs to provide minor variations of commands.

I'm pretty sure `vc-switches' is the usual way to provide invocation 
arguments variations in VC, or else there would be no such function.

> Or maybe do both, for those who might want this optional behavior
> always (although given the massive slow-down, and the general
> dismissal of the issue among git users, I doubt that such a person
> exists).

I believe you should rather customize the new variable once and for all 
for your preference, or set it on by-project basis, in .dir-locals.el.

Message #31 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Glenn Morris <rgm <at> gnu.org>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 18:47:10 +0200

On 02/24/2015 09:39 AM, Glenn Morris wrote:
>
> I find vc-annotate _unusably_ slow on the Emacs repo thanks to these -"C"s.
> Eg for rmailsum.el: 4 sec versus 5.5 minutes.

It takes ~40 sec with '-C -C' (as opposed to <1 sec) on my machine, with 
a fast SSD and Git 2.1.0. So it'd say it's usable.

The difference, I agree, is pretty striking, but rmailsum.el seems to be 
one of the worst cases.

Message #34 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 12:56:01 -0500

Dmitry Gutov wrote:

> On 02/24/2015 09:39 AM, Glenn Morris wrote:
>>
>> I find vc-annotate _unusably_ slow on the Emacs repo thanks to these -"C"s.
>> Eg for rmailsum.el: 4 sec versus 5.5 minutes.
>
> It takes ~40 sec with '-C -C' (as opposed to <1 sec) on my machine,
> with a fast SSD and Git 2.1.0. So it'd say it's usable.

Great, please send me your machine by soonest post. :)
It's unusable for me, and as Stefan said earlier, Emacs should follow
git's defaults.

Message #37 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Óscar Fuentes <ofv <at> wanadoo.es>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 12:57:57 -0500

Óscar Fuentes wrote:

> So what about this patch?

Looks, right, except vc-annotate-switches does not exist and would also
need to be added. And for consistency, presumably all backends should
get the same treatment. Thanks.

Message #40 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 20:04:49 +0200

On 02/24/2015 07:56 PM, Glenn Morris wrote:

> Great, please send me your machine by soonest post. :)

I'll get right on that. :)

> It's unusable for me,

Maybe you could also benefit from upgrading your Git, you've never 
mentioned the version.

> and as Stefan said earlier, Emacs should follow
> git's defaults.

Anyway, no argument from me here.

Message #43 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Glenn Morris <rgm <at> gnu.org>, Óscar Fuentes <ofv <at> wanadoo.es>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 22:55:20 +0200

On 02/24/2015 07:57 PM, Glenn Morris wrote:

> except vc-annotate-switches does not exist and would also
> need to be added.

I think this is only justified if we expect different backends' annotate 
command to receive the same options.

So far, only '-w' looks a likely candidate, albeit its descriptions are 
a bit different between Git and Hg. And it's not in e.g. Bzr.

What if the user sets `vc-annotate-switches' to '-w', and then calls 
`vc-annotate' in a Bazaar repository, where it'll obviously lead to failure?

> And for consistency, presumably all backends should
> get the same treatment. Thanks.

Probably. But I'd rather the present patch gets installed without too 
much delay.

Message #46 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Glenn Morris <rgm <at> gnu.org>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Tue, 24 Feb 2015 23:25:07 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 02/24/2015 07:57 PM, Glenn Morris wrote:
>
>> except vc-annotate-switches does not exist and would also
>> need to be added.
>
> I think this is only justified if we expect different backends'
> annotate command to receive the same options.

Actually, I implemented vc-annotate-switches too, but at the end
excluded it from the patch for the reason you mentioned.

Message #49 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Glenn Morris <rgm <at> gnu.org>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Wed, 25 Feb 2015 18:46:12 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 02/24/2015 07:57 PM, Glenn Morris wrote:
>
>> except vc-annotate-switches does not exist and would also
>> need to be added.
>
> I think this is only justified if we expect different backends'
> annotate command to receive the same options.

As the docstrings of vc-BACKEND-annotate-switches mention
vc-annotate-switches *and* vc-switches checks vc-annotate-switches, I
changed my mind again and implemented vc-annotate-switches with a caveat
on the docstring. Please review and suggest a better wording, if
necessary:

(defcustom vc-annotate-switches nil
  "A string or list of strings specifying switches for annotate under VC.
When running annotate under a given BACKEND, VC uses the first
non-nil value of `vc-BACKEND-annotate-switches', `vc-annotate-switches',
and `annotate-switches', in that order.  Since nil means to check the
next variable in the sequence, either of the first two may use
the value t to mean no switches at all.  `vc-annotate-switches'
should contain switches that are specific to version control, but
not specific to any particular backend.

As very few switches (if any) are used across different VC tools,
please consider using the specific `vc-BACKEND-annotate-switches'
for the backend you use."
  :type '(choice (const :tag "Unspecified" nil)
		 (const :tag "None" t)
		 (string :tag "Argument String")
		 (repeat :tag "Argument List" :value ("") string))
  :group 'vc
  :version "25.1")


> So far, only '-w' looks a likely candidate, albeit its descriptions
> are a bit different between Git and Hg. And it's not in e.g. Bzr.
>
> What if the user sets `vc-annotate-switches' to '-w', and then calls
> `vc-annotate' in a Bazaar repository, where it'll obviously lead to
> failure?
>
>> And for consistency, presumably all backends should
>> get the same treatment. Thanks.
>
> Probably. But I'd rather the present patch gets installed without too
> much delay.

I extended the vc-switches support to the rest of backends that
implement `vc-BACKEND-annotate-command', except rcs, that does some
complicated things (and I have no easy way of testing it.)

Once we agree on the resolution of the vc-annotate-switches issue, I'll
commit the changes.

Message #52 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Óscar Fuentes <ofv <at> wanadoo.es>
Cc: Glenn Morris <rgm <at> gnu.org>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Wed, 25 Feb 2015 19:58:36 +0200

On 02/25/2015 07:46 PM, Óscar Fuentes wrote:

> As the docstrings of vc-BACKEND-annotate-switches mention
> vc-annotate-switches *and* vc-switches checks vc-annotate-switches,

How about avoiding the mention of `vc-annotate-switches' in those 
docstrings instead?

Message #55 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Glenn Morris <rgm <at> gnu.org>, 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Wed, 25 Feb 2015 19:06:23 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 02/25/2015 07:46 PM, Óscar Fuentes wrote:
>
>> As the docstrings of vc-BACKEND-annotate-switches mention
>> vc-annotate-switches *and* vc-switches checks vc-annotate-switches,
>
> How about avoiding the mention of `vc-annotate-switches' in those
> docstrings instead?

vc-switches does check `vc-annotate-switches' (as it checks
vc-OP-switches for any other OP and BACKEND), so we are hiding
information from the user.

The whole problem consists on a generic approach (vc-OP-switches) that
doesn't play nice for some (all?) OPs. So we either get rid of
`vc-OP-switches' entirely or warn the user about its pitfalls.

(Is there an OP where `vc-OP-switches' works well across more than two
supported backends?)

Message #58 received at 17945 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Óscar Fuentes <ofv <at> wanadoo.es>
Cc: 17945 <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Wed, 25 Feb 2015 22:37:09 +0200

On 02/25/2015 08:06 PM, Óscar Fuentes wrote:

> vc-switches does check `vc-annotate-switches' (as it checks
> vc-OP-switches for any other OP and BACKEND), so we are hiding
> information from the user.

But there's no such user option. And if a variable with that name is 
created, well, the user stepped on something nobody asked them too.

> The whole problem consists on a generic approach (vc-OP-switches) that
> doesn't play nice for some (all?) OPs. So we either get rid of
> `vc-OP-switches' entirely or warn the user about its pitfalls.

I think in this case we can just as well avoid using `vc-switches' and 
reference `vc-git-annotate-switches' directly.

> (Is there an OP where `vc-OP-switches' works well across more than two
> supported backends?)

vc-diff-switches works pretty well.

The rest of -switches variables, though, look similarly suspicious to 
me. That's vc-checkin-switches, vc-checkout-switches and 
vc-register-switches.

Message #63 received at 17945-done <at> debbugs.gnu.org (full text, mbox):

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 17945-done <at> debbugs.gnu.org
Subject: Re: bug#17945: 24.4.50; vc-git-annotate-command is too slow
Date: Thu, 26 Feb 2015 15:56:05 +0100

Glenn Morris <rgm <at> gnu.org> writes:

> Óscar Fuentes wrote:
>
>> So what about this patch?
>
> Looks, right, except vc-annotate-switches does not exist and would also
> need to be added. And for consistency, presumably all backends should
> get the same treatment. Thanks.

Implemented on b5a0603eb41c7a350c16a1b3ec5c1b8d8c84a4eb.

I'm neutral about the existence of vc-annotate-switches, so if anyone
wants to remove it (including mentions on the docstrings of
vc-BACKEND-annotate-switches) it's fine with me.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.