Re: [PATCH] Findirect_function

[Manuel Giraud <manuel@ledu-giraud.fr>]

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

Hi,

So here are the results of `elisp-benchmarks-run' on one "emacs -Q"
master and one patched with the attached patch:

master:
* Results

  | test               | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | tot avg err (s) |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | inclist            |          28.16 |       0.00 |       0 |       28.16 |            0.80 |
  | fibn-rec           |          12.89 |       0.00 |       0 |       12.89 |            1.05 |
  | listlen-tc         |          12.68 |       0.00 |       0 |       12.68 |            0.47 |
  | pack-unpack        |           0.28 |       0.21 |       5 |        0.49 |            0.03 |
  | pidigits           |          32.88 |      45.67 |     211 |       78.55 |            0.67 |
  | map-closure        |          16.46 |       0.00 |       0 |       16.46 |            0.74 |
  | nbody              |           5.05 |      17.50 |     416 |       22.56 |            0.94 |
  | dhrystone          |          14.55 |       0.00 |       0 |       14.55 |            2.48 |
  | pack-unpack-old    |           0.75 |       0.47 |      11 |        1.22 |            0.16 |
  | bubble             |           8.54 |       9.90 |     234 |       18.44 |            1.61 |
  | inclist-type-hints |          28.42 |       0.00 |       0 |       28.42 |            0.55 |
  | bubble-no-cons     |          20.45 |       0.05 |       1 |       20.50 |            1.34 |
  | fibn               |          20.79 |       0.00 |       0 |       20.79 |            1.38 |
  | fibn-tc            |          11.13 |       0.00 |       0 |       11.13 |            0.41 |
  | pcase              |          21.63 |       0.00 |       0 |       21.63 |            0.25 |
  | flet               |          20.92 |       0.00 |       0 |       20.92 |            0.11 |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | total              |         255.58 |      73.80 |     878 |      329.38 |            4.10 |

patched:
* Results

  | test               | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | tot avg err (s) |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | inclist            |          27.89 |       0.00 |       0 |       27.89 |            0.20 |
  | fibn-rec           |          11.29 |       0.00 |       0 |       11.29 |            0.20 |
  | listlen-tc         |          12.71 |       0.00 |       0 |       12.71 |            0.98 |
  | pack-unpack        |           0.26 |       0.25 |      10 |        0.51 |            0.04 |
  | pidigits           |          32.27 |      45.05 |     382 |       77.32 |            1.37 |
  | map-closure        |          16.71 |       0.00 |       0 |       16.71 |            1.15 |
  | nbody              |           4.82 |      19.54 |     752 |       24.37 |            1.46 |
  | dhrystone          |          13.38 |       0.00 |       0 |       13.38 |            0.60 |
  | pack-unpack-old    |           0.72 |       0.51 |      20 |        1.23 |            0.08 |
  | bubble             |           8.12 |      10.19 |     400 |       18.30 |            0.92 |
  | inclist-type-hints |          28.40 |       0.00 |       0 |       28.40 |            0.69 |
  | bubble-no-cons     |          20.75 |       0.08 |       3 |       20.83 |            2.06 |
  | fibn               |          20.04 |       0.00 |       0 |       20.04 |            1.90 |
  | fibn-tc            |          11.02 |       0.00 |       0 |       11.02 |            1.03 |
  | pcase              |          22.31 |       0.00 |       0 |       22.31 |            2.14 |
  | flet               |          21.81 |       0.00 |       0 |       21.81 |            2.18 |
  |--------------------+----------------+------------+---------+-------------+-----------------|
  | total              |         252.51 |      75.61 |    1568 |      328.13 |            5.13 |

It seems that we are in the same ballpark.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-unused-argument-to-Findirect_function.patch --]
[-- Type: text/x-patch, Size: 5782 bytes --]

From c0a250642dc998c35e8a3d471198fb13ff5bb54f Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Mon, 17 Jan 2022 11:16:09 +0100
Subject: [PATCH] Remove unused argument to Findirect_function.

Use Findirect_function more where possible.
---
 src/callint.c |  2 +-
 src/data.c    |  8 +++-----
 src/doc.c     |  2 +-
 src/eval.c    | 36 ++++++++++--------------------------
 src/keymap.c  |  2 +-
 5 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/src/callint.c b/src/callint.c
index ce77c893f4..f046d53e95 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -327,7 +327,7 @@ DEFUN ("call-interactively", Fcall_interactively, Scall_interactively, 1, 3, 0,
   /* If SPECS is not a string, invent one.  */
   if (! STRINGP (specs))
     {
-      Lisp_Object funval = Findirect_function (function, Qt);
+      Lisp_Object funval = Findirect_function (function);
       uintmax_t events = num_input_events;
       Lisp_Object input = specs;
       /* Compute the arg values using the user's expression.  */
diff --git a/src/data.c b/src/data.c
index f287c38fcd..173f0fb54f 100644
--- a/src/data.c
+++ b/src/data.c
@@ -2365,13 +2365,13 @@ indirect_function (register Lisp_Object object)
   return hare;
 }
 
-DEFUN ("indirect-function", Findirect_function, Sindirect_function, 1, 2, 0,
+DEFUN ("indirect-function", Findirect_function, Sindirect_function, 1, 1, 0,
        doc: /* Return the function at the end of OBJECT's function chain.
 If OBJECT is not a symbol, just return it.  Otherwise, follow all
 function indirections to find the final function binding and return it.
 Signal a cyclic-function-indirection error if there is a loop in the
 function chain of symbols.  */)
-  (register Lisp_Object object, Lisp_Object noerror)
+  (register Lisp_Object object)
 {
   Lisp_Object result;
 
@@ -2380,10 +2380,8 @@ DEFUN ("indirect-function", Findirect_function, Sindirect_function, 1, 2, 0,
   if (SYMBOLP (result) && !NILP (result)
       && (result = XSYMBOL (result)->u.s.function, SYMBOLP (result)))
     result = indirect_function (result);
-  if (!NILP (result))
-    return result;
 
-  return Qnil;
+  return result;
 }
 \f
 /* Extract and set vector and string elements.  */
diff --git a/src/doc.c b/src/doc.c
index 0b12eb154d..3c8bb19eb6 100644
--- a/src/doc.c
+++ b/src/doc.c
@@ -325,7 +325,7 @@ DEFUN ("documentation", Fdocumentation, Sdocumentation, 1, 2, 0,
 					raw);
     }
 
-  Lisp_Object fun = Findirect_function (function, Qnil);
+  Lisp_Object fun = Findirect_function (function);
   if (NILP (fun))
     xsignal1 (Qvoid_function, function);
   if (CONSP (fun) && EQ (XCAR (fun), Qmacro))
diff --git a/src/eval.c b/src/eval.c
index 5514583b6a..a8c69f43ee 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2344,7 +2344,7 @@ DEFUN ("autoload-do-load", Fautoload_do_load, Sautoload_do_load, 1, 3, 0,
     return Qnil;
   else
     {
-      Lisp_Object fun = Findirect_function (funname, Qnil);
+      Lisp_Object fun = Findirect_function(funname);
 
       if (!NILP (Fequal (fun, fundef)))
 	error ("Autoloading file %s failed to define function %s",
@@ -2667,15 +2667,10 @@ DEFUN ("apply", Fapply, Sapply, 1, MANY, 0,
 
   numargs += nargs - 2;
 
-  /* Optimize for no indirection.  */
-  if (SYMBOLP (fun) && !NILP (fun)
-      && (fun = XSYMBOL (fun)->u.s.function, SYMBOLP (fun)))
-    {
-      fun = indirect_function (fun);
-      if (NILP (fun))
-	/* Let funcall get the error.  */
-	fun = args[0];
-    }
+  fun = Findirect_function(fun);
+  if (NILP (fun))
+    /* Let funcall get the error.  */
+    fun = args[0];
 
   if (SUBRP (fun) && XSUBR (fun)->max_args > numargs
       /* Don't hide an error by adding missing arguments.  */
@@ -3008,7 +3003,7 @@ FUNCTIONP (Lisp_Object object)
 {
   if (SYMBOLP (object) && !NILP (Ffboundp (object)))
     {
-      object = Findirect_function (object, Qt);
+      object = Findirect_function(object);
 
       if (CONSP (object) && EQ (XCAR (object), Qautoload))
 	{
@@ -3068,11 +3063,7 @@ DEFUN ("funcall", Ffuncall, Sfuncall, 1, MANY, 0,
 
  retry:
 
-  /* Optimize for no indirection.  */
-  fun = original_fun;
-  if (SYMBOLP (fun) && !NILP (fun)
-      && (fun = XSYMBOL (fun)->u.s.function, SYMBOLP (fun)))
-    fun = indirect_function (fun);
+  fun = Findirect_function(original_fun);
 
   if (SUBRP (fun) && !SUBR_NATIVE_COMPILED_DYNP (fun))
     val = funcall_subr (XSUBR (fun), numargs, args + 1);
@@ -3387,14 +3378,7 @@ DEFUN ("func-arity", Ffunc_arity, Sfunc_arity, 1, 1, 0,
 
  retry:
 
-  /* Optimize for no indirection.  */
-  function = original;
-  if (SYMBOLP (function) && !NILP (function))
-    {
-      function = XSYMBOL (function)->u.s.function;
-      if (SYMBOLP (function))
-	function = indirect_function (function);
-    }
+  function = Findirect_function(original);
 
   if (CONSP (function) && EQ (XCAR (function), Qmacro))
     function = XCDR (function);
@@ -3936,9 +3920,9 @@ get_backtrace_starting_at (Lisp_Object base)
 
   if (!NILP (base))
     { /* Skip up to `base'.  */
-      base = Findirect_function (base, Qt);
+      base = Findirect_function(base);
       while (backtrace_p (pdl)
-             && !EQ (base, Findirect_function (backtrace_function (pdl), Qt)))
+             && !EQ (base, Findirect_function(backtrace_function (pdl))))
         pdl = backtrace_next (pdl);
     }
 
diff --git a/src/keymap.c b/src/keymap.c
index ed69b1c427..7be0e3c155 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -1614,7 +1614,7 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr)
 	      }
 
 	    /* Get the keymap definition--or nil if it is not defined.  */
-	    temp = Findirect_function (XCDR (assoc), Qt);
+	    temp = Findirect_function(XCDR (assoc));
 	    if (!NILP (temp))
 	      {
 		cmm_modes[i] = var;
-- 
2.34.1


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

-- 
Manuel Giraud