[harder@ifa.au.dk: Speed of all-completions]

[harder@ifa.au.dk: Speed of all-completions]

[Richard Stallman]

Would someone please take a look at this?

------- Start of forwarded message -------
To: emacs-pretest-bug@gnu.org
From: Jesper Harder <harder@ifa.au.dk>
Date: Sun, 30 May 2004 17:37:56 +0200
Subject: Speed of all-completions
Sender: emacs-pretest-bug-bounces+rms=gnu.org@gnu.org
X-Spam-Status: No, hits=-0.5 required=5.0
	tests=USER_AGENT_GNUS_UA
	version=2.55
X-Spam-Level: 
X-Spam-Checker-Version: SpamAssassin 2.55 (1.174.2.19-2003-05-19-exp)

In GNU Emacs 21.3.50.10 (i686-pc-linux-gnu, GTK+ Version 2.0.2)
 of 2004-05-25 on defun.localdomain

I use icomplete-mode and I've noticed that it feels more sluggish in
cvs Emacs than in Emacs 21.3.

Profiling shows that the bulk of the time is spent in
`all-completions', so I tried to benchmark it with this example:

(defmacro time (form)
  `(let ((t1 (float-time)))
    ,form
    (- (float-time) t1)))

(let ((oba (make-vector 255 0)))
  (dotimes (i 10000)
    (intern (format "f%i" i) oba))
  (time
   (all-completions "f" oba)))

The time used was:

  cvs Emacs:   0.16448211669921875
  Emacs 21.3:  0.01507115364074707

i.e. `all-completions' is roughly an order of magnitude slower in cvs
Emacs.

- -- 
Jesper Harder                                <http://purl.org/harder/>


_______________________________________________
Emacs-pretest-bug mailing list
Emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

> Would someone please take a look at this?
>
> From: Jesper Harder <harder@ifa.au.dk>
> Subject: Speed of all-completions
> To: emacs-pretest-bug@gnu.org
> Date: Sun, 30 May 2004 17:37:56 +0200
> X-Sent: 2 weeks, 6 hours, 17 minutes, 49 seconds ago
>
> In GNU Emacs 21.3.50.10 (i686-pc-linux-gnu, GTK+ Version 2.0.2)
>  of 2004-05-25 on defun.localdomain
>
> I use icomplete-mode and I've noticed that it feels more sluggish in
> cvs Emacs than in Emacs 21.3.
>
> Profiling shows that the bulk of the time is spent in
> `all-completions', so I tried to benchmark it with this example:
>
> (defmacro time (form)
>   `(let ((t1 (float-time)))
>     ,form
>     (- (float-time) t1)))
>
> (let ((oba (make-vector 255 0)))
>   (dotimes (i 10000)
>     (intern (format "f%i" i) oba))
>   (time
>    (all-completions "f" oba)))
>
> The time used was:
>
>   cvs Emacs:   0.16448211669921875
>   Emacs 21.3:  0.01507115364074707

This is due to this change:

	(Ftry_completion, Fall_completions, Ftest_completion): Bind
	case-fold-search to the value of completion-ignore-case when
	checking completion-regexp-list.

I've checked in a fix that avoids the overhead of specbind when
completion-regexp-list is empty.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> This is due to this change:
> 
> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
> 	case-fold-search to the value of completion-ignore-case when
> 	checking completion-regexp-list.
> 
> I've checked in a fix that avoids the overhead of specbind when
> completion-regexp-list is empty.

At the cost of being more expensive when it isn't.  I don't see why
you have to undo the binding all the time.  Just initialize the
binding depth variable to something illegal at the start of the
function.  Then when you need the binding, you check whether it has
already been done and do it if needed.

And at the exit of the function, you check whether the binding had
happened and in that case undo it before returning.

That way you avoid the overhead of the binding when it is not needed,
but pay at most for one binding when it _is_ needed.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Luc Teirlinck]

David Kastrup wrote:
   
   Andreas Schwab <schwab@suse.de> writes:

   > This is due to this change:
   > 
   > 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
   > 	case-fold-search to the value of completion-ignore-case when
   > 	checking completion-regexp-list.
   > 
   > I've checked in a fix that avoids the overhead of specbind when
   > completion-regexp-list is empty.

   At the cost of being more expensive when it isn't.  I don't see why
   you have to undo the binding all the time.  Just initialize the
   binding depth variable to something illegal at the start of the
   function.  Then when you need the binding, you check whether it has
   already been done and do it if needed.

   And at the exit of the function, you check whether the binding had
   happened and in that case undo it before returning.

I believe that it is completely safe to bind case-fold-search to the
value of completion-ignore-case _once_ around the entire while loops
in Ftry_completion and Fall_completions.  No minibuffer is entered, so
there is no need to restore any user customized value of
case-fold-search.  I do not see the problem in Ftest_completion,
because the binding seems to be made only once there anyway.

For the moment, I have just taken a quick look.  I could double-check
this in more detail if that is what we want to do.

I believe I was just excessively conservative when I committed the
original patch, because I did not realize just how expensive specbind
was.

Note that there is only one single place in the entire Emacs C and
Lisp code where completion-regexp-list is non-nil, namely in
customize-mode.  So it is definitely more important to optimize in the
case where it is nil, than in the case where it is not.

Sincerely,

Luc.

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> David Kastrup wrote:
>    
>    Andreas Schwab <schwab@suse.de> writes:
> 
>    > This is due to this change:
>    > 
>    > 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
>    > 	case-fold-search to the value of completion-ignore-case when
>    > 	checking completion-regexp-list.
>    > 
>    > I've checked in a fix that avoids the overhead of specbind when
>    > completion-regexp-list is empty.
> 
>    At the cost of being more expensive when it isn't.  I don't see why
>    you have to undo the binding all the time.  Just initialize the
>    binding depth variable to something illegal at the start of the
>    function.  Then when you need the binding, you check whether it has
>    already been done and do it if needed.
> 
>    And at the exit of the function, you check whether the binding had
>    happened and in that case undo it before returning.
> 
> I believe that it is completely safe to bind case-fold-search to the
> value of completion-ignore-case _once_ around the entire while loops
> in Ftry_completion and Fall_completions.

Unless I misunderstand, that is what Andreas replaced, since the
"once" cost occured far too often.

So he changed once always to once per loop.  I was proposing once at
most.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> This is due to this change:
>> 
>> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
>> 	case-fold-search to the value of completion-ignore-case when
>> 	checking completion-regexp-list.
>> 
>> I've checked in a fix that avoids the overhead of specbind when
>> completion-regexp-list is empty.
>
> At the cost of being more expensive when it isn't.

Why is it more expensive?  The check for CONSP (completion-regexp-list)
has to be done anyway, and can be CSE'd by the compiler.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > Andreas Schwab <schwab@suse.de> writes:
> >
> >> This is due to this change:
> >> 
> >> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
> >> 	case-fold-search to the value of completion-ignore-case when
> >> 	checking completion-regexp-list.
> >> 
> >> I've checked in a fix that avoids the overhead of specbind when
> >> completion-regexp-list is empty.
> >
> > At the cost of being more expensive when it isn't.
> 
> Why is it more expensive?  The check for CONSP (completion-regexp-list)
> has to be done anyway, and can be CSE'd by the compiler.

Because you are redoing the binding for every element in the loop,
while it is needed only once.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> David Kastrup <dak@gnu.org> writes:
>> 
>> > Andreas Schwab <schwab@suse.de> writes:
>> >
>> >> This is due to this change:
>> >> 
>> >> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
>> >> 	case-fold-search to the value of completion-ignore-case when
>> >> 	checking completion-regexp-list.
>> >> 
>> >> I've checked in a fix that avoids the overhead of specbind when
>> >> completion-regexp-list is empty.
>> >
>> > At the cost of being more expensive when it isn't.
>> 
>> Why is it more expensive?  The check for CONSP (completion-regexp-list)
>> has to be done anyway, and can be CSE'd by the compiler.
>
> Because you are redoing the binding for every element in the loop,
> while it is needed only once.

I did not change anything in this regard.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

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

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > Andreas Schwab <schwab@suse.de> writes:
> >
> >> David Kastrup <dak@gnu.org> writes:
> >> 
> >> > Andreas Schwab <schwab@suse.de> writes:
> >> >
> >> >> This is due to this change:
> >> >> 
> >> >> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
> >> >> 	case-fold-search to the value of completion-ignore-case when
> >> >> 	checking completion-regexp-list.
> >> >> 
> >> >> I've checked in a fix that avoids the overhead of specbind when
> >> >> completion-regexp-list is empty.
> >> >
> >> > At the cost of being more expensive when it isn't.
> >> 
> >> Why is it more expensive?  The check for CONSP (completion-regexp-list)
> >> has to be done anyway, and can be CSE'd by the compiler.
> >
> > Because you are redoing the binding for every element in the loop,
> > while it is needed only once.
> 
> I did not change anything in this regard.

Serves me right from just looking at the result instead of what you
started with.

To illustrate what I mean, here is a patch against the most recent
version.  Note that I have to undo the binding before calling a
user-defined predicate function (which might rely on the original
binding).  The last of your three changes, not running in a loop, was
not improvable, however.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3633 bytes --]

--- minibuf.c	15 Jun 2004 00:47:07 +0200	1.269
+++ minibuf.c	15 Jun 2004 15:22:48 +0200	
@@ -1207,6 +1207,7 @@
 			   || NILP (XCAR (alist))));
   int index = 0, obsize = 0;
   int matchcount = 0;
+  int bind_count = -1;
   Lisp_Object bucket, zero, end, tem;
   struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
 
@@ -1285,21 +1286,22 @@
 	  XSETFASTINT (zero, 0);
 
 	  /* Ignore this element if it fails to match all the regexps.  */
-	  if (CONSP (Vcompletion_regexp_list))
-	    {
-	      int count = SPECPDL_INDEX ();
-	      specbind (Qcase_fold_search, completion_ignore_case ? Qt : Qnil);
-	      for (regexps = Vcompletion_regexp_list; CONSP (regexps);
-		   regexps = XCDR (regexps))
-		{
-		  tem = Fstring_match (XCAR (regexps), eltstring, zero);
-		  if (NILP (tem))
-		    break;
+	  {
+	    for (regexps = Vcompletion_regexp_list; CONSP (regexps);
+		 regexps = XCDR (regexps))
+	      {
+		if (bind_count < 0) {
+		  bind_count = SPECPDL_INDEX ();
+		  specbind (Qcase_fold_search,
+			    completion_ignore_case ? Qt : Qnil);
 		}
-	      unbind_to (count, Qnil);
-	      if (CONSP (regexps))
-		continue;
-	    }
+		tem = Fstring_match (XCAR (regexps), eltstring, zero);
+		if (NILP (tem))
+		  break;
+	      }
+	    if (CONSP (regexps))
+	      continue;
+	  }
 
 	  /* Ignore this element if there is a predicate
 	     and the predicate doesn't like it. */
@@ -1310,6 +1312,10 @@
 		tem = Fcommandp (elt, Qnil);
 	      else
 		{
+		  if (bind_count >= 0) {
+		    unbind_to (bind_count, Qnil);
+		    bind_count = -1;
+		  }
 		  GCPRO4 (tail, string, eltstring, bestmatch);
 		  tem = type == 3
 		    ? call2 (predicate, elt,
@@ -1391,6 +1397,11 @@
 	}
     }
 
+  if (bind_count >= 0) {
+    unbind_to (bind_count, Qnil);
+    bind_count = -1;
+  }
+
   if (NILP (bestmatch))
     return Qnil;		/* No completions found */
   /* If we are ignoring case, and there is no exact match,
@@ -1453,6 +1464,7 @@
 		       && (!SYMBOLP (XCAR (alist))
 			   || NILP (XCAR (alist))));
   int index = 0, obsize = 0;
+  int bind_count = -1;
   Lisp_Object bucket, tem;
   struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
 
@@ -1537,21 +1549,22 @@
 	  XSETFASTINT (zero, 0);
 
 	  /* Ignore this element if it fails to match all the regexps.  */
-	  if (CONSP (Vcompletion_regexp_list))
-	    {
-	      int count = SPECPDL_INDEX ();
-	      specbind (Qcase_fold_search, completion_ignore_case ? Qt : Qnil);
-	      for (regexps = Vcompletion_regexp_list; CONSP (regexps);
-		   regexps = XCDR (regexps))
-		{
-		  tem = Fstring_match (XCAR (regexps), eltstring, zero);
-		  if (NILP (tem))
-		    break;
+	  {
+	    for (regexps = Vcompletion_regexp_list; CONSP (regexps);
+		 regexps = XCDR (regexps))
+	      {
+		if (bind_count < 0) {
+		  bind_count = SPECPDL_INDEX ();
+		  specbind (Qcase_fold_search,
+			    completion_ignore_case ? Qt : Qnil);
 		}
-	      unbind_to (count, Qnil);
-	      if (CONSP (regexps))
-		continue;
-	    }
+		tem = Fstring_match (XCAR (regexps), eltstring, zero);
+		if (NILP (tem))
+		  break;
+	      }
+	    if (CONSP (regexps))
+	      continue;
+	  }
 
 	  /* Ignore this element if there is a predicate
 	     and the predicate doesn't like it. */
@@ -1562,6 +1575,10 @@
 		tem = Fcommandp (elt, Qnil);
 	      else
 		{
+		  if (bind_count >= 0) {
+		    unbind_to (bind_count, Qnil);
+		    bind_count = -1;
+		  }
 		  GCPRO4 (tail, eltstring, allmatches, string);
 		  tem = type == 3
 		    ? call2 (predicate, elt,
@@ -1576,6 +1593,11 @@
 	}
     }
 
+  if (bind_count >= 0) {
+    unbind_to (bind_count, Qnil);
+    bind_count = -1;
+  }
+
   return Fnreverse (allmatches);
 }
 \f

[-- Attachment #3: Type: text/plain, Size: 80 bytes --]


Does this look sane enough?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> To illustrate what I mean, here is a patch against the most recent
> version.  Note that I have to undo the binding before calling a
> user-defined predicate function (which might rely on the original
> binding).  The last of your three changes, not running in a loop, was
                         ^^^^^^^^^^
> not improvable, however.

Where do you get this idea?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > To illustrate what I mean, here is a patch against the most recent
> > version.  Note that I have to undo the binding before calling a
> > user-defined predicate function (which might rely on the original
> > binding).  The last of your three changes, not running in a loop, was
>                          ^^^^^^^^^^
> > not improvable, however.
> 
> Where do you get this idea?

From: Andreas Schwab <schwab@suse.de>
Date: Mon, 14 Jun 2004 00:22:13 +0200
Message-ID: <jehdtfp08a.fsf@sykes.suse.de>

[...]

This is due to this change:

	(Ftry_completion, Fall_completions, Ftest_completion): Bind
	case-fold-search to the value of completion-ignore-case when
	checking completion-regexp-list.

I've checked in a fix that avoids the overhead of specbind when
completion-regexp-list is empty.

Andreas.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> David Kastrup <dak@gnu.org> writes:
>> 
>> > To illustrate what I mean, here is a patch against the most recent
>> > version.  Note that I have to undo the binding before calling a
>> > user-defined predicate function (which might rely on the original
>> > binding).  The last of your three changes, not running in a loop, was
>>                          ^^^^^^^^^^
>> > not improvable, however.
>> 
>> Where do you get this idea?
>
> From: Andreas Schwab <schwab@suse.de>
> Date: Mon, 14 Jun 2004 00:22:13 +0200
> Message-ID: <jehdtfp08a.fsf@sykes.suse.de>
>
> [...]
>
> This is due to this change:
>
> 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
> 	case-fold-search to the value of completion-ignore-case when
> 	checking completion-regexp-list.

This is not my change.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
> 
> > Andreas Schwab <schwab@suse.de> writes:
> >
> >> David Kastrup <dak@gnu.org> writes:
> >> 
> >> > To illustrate what I mean, here is a patch against the most recent
> >> > version.  Note that I have to undo the binding before calling a
> >> > user-defined predicate function (which might rely on the original
> >> > binding).  The last of your three changes, not running in a loop, was
> >>                          ^^^^^^^^^^
> >> > not improvable, however.
> >> 
> >> Where do you get this idea?
> >
> > From: Andreas Schwab <schwab@suse.de>
> > Date: Mon, 14 Jun 2004 00:22:13 +0200
> > Message-ID: <jehdtfp08a.fsf@sykes.suse.de>
> >
> > [...]
> >
> > This is due to this change:
> >
> > 	(Ftry_completion, Fall_completions, Ftest_completion): Bind
> > 	case-fold-search to the value of completion-ignore-case when
> > 	checking completion-regexp-list.
> 
> This is not my change.

I did not claim it was.  You conveniently snipped the rest of the
mail where you explain your fix to the above change.

There is an entry in the src/ChangeLog from you:

2004-06-14  Andreas Schwab  <schwab@suse.de>

	* minibuf.c (Ftry_completion, Fall_completions, Ftest_completion):
	Avoid calling specbind when completion-regexp-list is empty.

Can we now stop this silliness?  You had three changes, one in
Ftry_completion, one in Fall_completions, one in Ftest_completion.
You sent mail explaining the changes and you entered them in
src/ChangeLog.  I posted an amendment and explained the reason for it.

If you want to deny that you made those changes, that's fine with me,
but it adds nothing to the discussion about the viability of the
improvement I proposed to the changes somebody happened to make under
your name.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Can we now stop this silliness?  You had three changes, one in
> Ftry_completion, one in Fall_completions, one in Ftest_completion.

Sorry, a misunderstanding.  I regard this as one single (logical) change,
so I thought you were talking about something else.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [harder@ifa.au.dk: Speed of all-completions]

[Luc Teirlinck]

David Kastrup wrote:

   To illustrate what I mean, here is a patch against the most recent
   version.  Note that I have to undo the binding before calling a
   user-defined predicate function (which might rely on the original
   binding).  The last of your three changes, not running in a loop, was
   not improvable, however.

I believe it is a programmer defined predicate function.  The obvious
alternative to your patch would be to have case-fold-search bound to
the value of completion-ignore-case around the entire while loops.
The difference with the current situation would be that the binding
would be in effect during the call to the predicate function.  I
believe that the result of the predicate function should not depend on
a value which the user customized for a completely unrelated reason,
interactive convenience.  So the binding probably _should_ be in effect
during the call to PREDICATE.

If we decide to bind around the entire while loop we should document
the fact that the binding is in effect during the call to predicate in
the Elisp manual.  In that case, the
`if (CONSP (Vcompletion_regexp_list))' should be removed, and for
consistency, the scope should also be extended in Ftest_completion.

Is one single prolonged binding using specbind expensive or was the
efficiency problem completely caused by the fact that it was used
countless times in a loop?

Sincerely,

Luc.

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> If we decide to bind around the entire while loop we should document
> the fact that the binding is in effect during the call to predicate in
> the Elisp manual.  In that case, the
> `if (CONSP (Vcompletion_regexp_list))' should be removed, and for
> consistency, the scope should also be extended in Ftest_completion.
> 
> Is one single prolonged binding using specbind expensive or was the
> efficiency problem completely caused by the fact that it was used
> countless times in a loop?

The single prolonged binding is expensive, as in most cases the loop
is not entered even once.

At least that's what I understood.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [harder@ifa.au.dk: Speed of all-completions]

[Richard Stallman]

Thanks for fixing this.

Re: [harder@ifa.au.dk: Speed of all-completions]

[Luc Teirlinck]

David Kastrup wrote:

   > Is one single prolonged binding using specbind expensive or was the
   > efficiency problem completely caused by the fact that it was used
   > countless times in a loop?

   The single prolonged binding is expensive, as in most cases the loop
   is not entered even once.

   At least that's what I understood.

Does not look like it.  I could provide a patch and timings showing
that with one single binding around essentially the entire functions
and _without_ the `if (CONSP (Vcompletion_regexp_list))', there is a
10-fold improvement (in Jesper's example), if completion-regexp-list
is non-nil and a negligible deterioration if it is nil (everything
compared with the current CVS code).

However, before going into that, something else needs to be looked at.

Binding (essentially) around the entire functions means that the
binding is also in effect around the call to PREDICATE.  Actually, if
PREDICATE uses case-fold-search, the binding either _should_ be in
effect, or PREDICATE should explicitly bind case-fold-search itself.

The _one single_ problem with the one prolonged binding might occur if
PREDICATE asks questions to the user in the minibuffer.  We have
forgotten the user defined value and it can not be restored.  But it
should be restored, because otherwise the user might be confused.

It probably is possible to use all-completions and friends in an
unusual way (that is, for purposes other than minibuffer completion),
where PREDICATE _could_ ask minibuffer questions.  _If so_, then we
either have to stick with the present version or apply your patch.

Sincerely,

Luc.

Re: [harder@ifa.au.dk: Speed of all-completions]

[Richard Stallman]

    It probably is possible to use all-completions and friends in an
    unusual way (that is, for purposes other than minibuffer completion),
    where PREDICATE _could_ ask minibuffer questions.  _If so_, then we
    either have to stick with the present version or apply your patch.

Let's apply that patch and stop discussing this, so we can spend
our time fixing other bugs, or working (for after the next release)
on some of the items in etc/TODO, or something else useful.

Re: [harder@ifa.au.dk: Speed of all-completions]

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     It probably is possible to use all-completions and friends in an
>     unusual way (that is, for purposes other than minibuffer completion),
>     where PREDICATE _could_ ask minibuffer questions.  _If so_, then we
>     either have to stick with the present version or apply your patch.
> 
> Let's apply that patch and stop discussing this, so we can spend
> our time fixing other bugs, or working (for after the next release)
> on some of the items in etc/TODO, or something else useful.

I applied the patch (after renaming bind_count to bindcount for
consistency with other non-Lisp-accessible variables).  Should be
close to fast in most cases, I guess; and between the two basic
choices of just dropping the work and committing it, the latter seemed
saner.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum