unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#17945: 24.4.50; vc-git-annotate-command is too slow
@ 2014-07-05 12:28 William Xu
  2014-07-05 13:34 ` Dmitry
  0 siblings, 1 reply; 19+ messages in thread
From: William Xu @ 2014-07-05 12:28 UTC (permalink / raw)
  To: 17945

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 995 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2014-07-05 12:28 bug#17945: 24.4.50; vc-git-annotate-command is too slow William Xu
@ 2014-07-05 13:34 ` Dmitry
  2014-07-05 15:16   ` William Xu
  2014-07-05 16:15   ` Stefan Monnier
  0 siblings, 2 replies; 19+ messages in thread
From: Dmitry @ 2014-07-05 13:34 UTC (permalink / raw)
  To: William Xu; +Cc: 17945

William Xu <william.xwl@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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2014-07-05 13:34 ` Dmitry
@ 2014-07-05 15:16   ` William Xu
  2014-07-05 16:15   ` Stefan Monnier
  1 sibling, 0 replies; 19+ messages in thread
From: William Xu @ 2014-07-05 15:16 UTC (permalink / raw)
  To: Dmitry; +Cc: 17945@debbugs.gnu.org

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

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

> William Xu <william.xwl@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

[-- Attachment #2: Type: text/html, Size: 1825 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2014-07-05 13:34 ` Dmitry
  2014-07-05 15:16   ` William Xu
@ 2014-07-05 16:15   ` Stefan Monnier
  2015-02-24  7:39     ` Glenn Morris
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2014-07-05 16:15 UTC (permalink / raw)
  To: Dmitry; +Cc: William Xu, 17945

>> 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





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2014-07-05 16:15   ` Stefan Monnier
@ 2015-02-24  7:39     ` Glenn Morris
  2015-02-24 15:31       ` Óscar Fuentes
  2015-02-24 16:47       ` Dmitry Gutov
  0 siblings, 2 replies; 19+ messages in thread
From: Glenn Morris @ 2015-02-24  7:39 UTC (permalink / raw)
  To: 17945


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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24  7:39     ` Glenn Morris
@ 2015-02-24 15:31       ` Óscar Fuentes
  2015-02-24 16:12         ` Eli Zaretskii
  2015-02-24 17:57         ` Glenn Morris
  2015-02-24 16:47       ` Dmitry Gutov
  1 sibling, 2 replies; 19+ messages in thread
From: Óscar Fuentes @ 2015-02-24 15:31 UTC (permalink / raw)
  To: 17945

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))






^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 15:31       ` Óscar Fuentes
@ 2015-02-24 16:12         ` Eli Zaretskii
  2015-02-24 16:36           ` Dmitry Gutov
  2015-02-24 17:57         ` Glenn Morris
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2015-02-24 16:12 UTC (permalink / raw)
  To: Óscar Fuentes; +Cc: 17945

> From: Óscar Fuentes <ofv@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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 16:12         ` Eli Zaretskii
@ 2015-02-24 16:36           ` Dmitry Gutov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-24 16:36 UTC (permalink / raw)
  To: Eli Zaretskii, Óscar Fuentes; +Cc: 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24  7:39     ` Glenn Morris
  2015-02-24 15:31       ` Óscar Fuentes
@ 2015-02-24 16:47       ` Dmitry Gutov
  2015-02-24 17:56         ` Glenn Morris
  1 sibling, 1 reply; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-24 16:47 UTC (permalink / raw)
  To: Glenn Morris, 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 16:47       ` Dmitry Gutov
@ 2015-02-24 17:56         ` Glenn Morris
  2015-02-24 18:04           ` Dmitry Gutov
  0 siblings, 1 reply; 19+ messages in thread
From: Glenn Morris @ 2015-02-24 17:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 15:31       ` Óscar Fuentes
  2015-02-24 16:12         ` Eli Zaretskii
@ 2015-02-24 17:57         ` Glenn Morris
  2015-02-24 20:55           ` Dmitry Gutov
  2015-02-26 14:56           ` Óscar Fuentes
  1 sibling, 2 replies; 19+ messages in thread
From: Glenn Morris @ 2015-02-24 17:57 UTC (permalink / raw)
  To: Óscar Fuentes; +Cc: 17945

Ó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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 17:56         ` Glenn Morris
@ 2015-02-24 18:04           ` Dmitry Gutov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-24 18:04 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 17:57         ` Glenn Morris
@ 2015-02-24 20:55           ` Dmitry Gutov
  2015-02-24 22:25             ` Óscar Fuentes
  2015-02-25 17:46             ` Óscar Fuentes
  2015-02-26 14:56           ` Óscar Fuentes
  1 sibling, 2 replies; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-24 20:55 UTC (permalink / raw)
  To: Glenn Morris, Óscar Fuentes; +Cc: 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 20:55           ` Dmitry Gutov
@ 2015-02-24 22:25             ` Óscar Fuentes
  2015-02-25 17:46             ` Óscar Fuentes
  1 sibling, 0 replies; 19+ messages in thread
From: Óscar Fuentes @ 2015-02-24 22:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 17945

Dmitry Gutov <dgutov@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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 20:55           ` Dmitry Gutov
  2015-02-24 22:25             ` Óscar Fuentes
@ 2015-02-25 17:46             ` Óscar Fuentes
  2015-02-25 17:58               ` Dmitry Gutov
  1 sibling, 1 reply; 19+ messages in thread
From: Óscar Fuentes @ 2015-02-25 17:46 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 17945

Dmitry Gutov <dgutov@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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-25 17:46             ` Óscar Fuentes
@ 2015-02-25 17:58               ` Dmitry Gutov
  2015-02-25 18:06                 ` Óscar Fuentes
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-25 17:58 UTC (permalink / raw)
  To: Óscar Fuentes; +Cc: 17945

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?





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-25 17:58               ` Dmitry Gutov
@ 2015-02-25 18:06                 ` Óscar Fuentes
  2015-02-25 20:37                   ` Dmitry Gutov
  0 siblings, 1 reply; 19+ messages in thread
From: Óscar Fuentes @ 2015-02-25 18:06 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 17945

Dmitry Gutov <dgutov@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?)





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-25 18:06                 ` Óscar Fuentes
@ 2015-02-25 20:37                   ` Dmitry Gutov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Gutov @ 2015-02-25 20:37 UTC (permalink / raw)
  To: Óscar Fuentes; +Cc: 17945

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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#17945: 24.4.50; vc-git-annotate-command is too slow
  2015-02-24 17:57         ` Glenn Morris
  2015-02-24 20:55           ` Dmitry Gutov
@ 2015-02-26 14:56           ` Óscar Fuentes
  1 sibling, 0 replies; 19+ messages in thread
From: Óscar Fuentes @ 2015-02-26 14:56 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 17945-done

Glenn Morris <rgm@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.





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-02-26 14:56 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-05 12:28 bug#17945: 24.4.50; vc-git-annotate-command is too slow William Xu
2014-07-05 13:34 ` Dmitry
2014-07-05 15:16   ` William Xu
2014-07-05 16:15   ` Stefan Monnier
2015-02-24  7:39     ` Glenn Morris
2015-02-24 15:31       ` Óscar Fuentes
2015-02-24 16:12         ` Eli Zaretskii
2015-02-24 16:36           ` Dmitry Gutov
2015-02-24 17:57         ` Glenn Morris
2015-02-24 20:55           ` Dmitry Gutov
2015-02-24 22:25             ` Óscar Fuentes
2015-02-25 17:46             ` Óscar Fuentes
2015-02-25 17:58               ` Dmitry Gutov
2015-02-25 18:06                 ` Óscar Fuentes
2015-02-25 20:37                   ` Dmitry Gutov
2015-02-26 14:56           ` Óscar Fuentes
2015-02-24 16:47       ` Dmitry Gutov
2015-02-24 17:56         ` Glenn Morris
2015-02-24 18:04           ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).