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

[David Kastrup <dak@gnu.org>]

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