unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Findirect_function
@ 2022-01-14 10:40 Manuel Giraud
  2022-01-14 12:09 ` Mattias Engdegård
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Manuel Giraud @ 2022-01-14 10:40 UTC (permalink / raw)
  To: emacs-devel

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

Hi,

Maybe it is bike shedding territory (and maybe I'm missing something)
but here are two patches for Findirect_function and its usage. "make
check" is the same and so far it works for me™.

Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-simplify-return-case-for-Findirect_function.patch --]
[-- Type: text/x-patch, Size: 805 bytes --]

From 300db08a4d66814000e2d6eb373f931c37913954 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 14 Jan 2022 11:28:28 +0100
Subject: [PATCH 1/2] simplify return case for Findirect_function.

---
 src/data.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/data.c b/src/data.c
index f287c38fcd..65babc49e9 100644
--- a/src/data.c
+++ b/src/data.c
@@ -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.  */
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-use-Findirect_function-more.patch --]
[-- Type: text/x-patch, Size: 1939 bytes --]

From 99658ecb6860185e0b3fdb8932de3a8b6f7e0f69 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 14 Jan 2022 11:29:10 +0100
Subject: [PATCH 2/2] use Findirect_function more.

---
 src/eval.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 5514583b6a..f72eb6f67a 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2668,14 +2668,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, Qt);
+  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.  */
@@ -3069,10 +3065,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, Qt);
 
   if (SUBRP (fun) && !SUBR_NATIVE_COMPILED_DYNP (fun))
     val = funcall_subr (XSUBR (fun), numargs, args + 1);
@@ -3388,13 +3381,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, Qt);
 
   if (CONSP (function) && EQ (XCAR (function), Qmacro))
     function = XCDR (function);
-- 
2.34.1


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

-- 
Manuel Giraud

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

* Re: [PATCH] Findirect_function
  2022-01-14 10:40 [PATCH] Findirect_function Manuel Giraud
@ 2022-01-14 12:09 ` Mattias Engdegård
  2022-01-14 13:00   ` Manuel Giraud
  2022-01-14 12:16 ` Eli Zaretskii
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Mattias Engdegård @ 2022-01-14 12:09 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

14 jan. 2022 kl. 11.40 skrev Manuel Giraud <manuel@ledu-giraud.fr>:

> Maybe it is bike shedding territory (and maybe I'm missing something)

Not at all, patches are always welcome!

> but here are two patches for Findirect_function and its usage. "make
> check" is the same and so far it works for me™.

The first change is straightforward:

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

Good! See if you can find more instances of this unfortunate pattern. Even if it makes no difference for the generated code (very likely), it obfuscates the source code.

The other changes are less obvious since they may introduce extra costs; some of these code paths are fairly hot. In addition, I'm giving part of the interpreter an overhaul so perhaps we could drop them for the time being. Do you agree?




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

* Re: [PATCH] Findirect_function
  2022-01-14 10:40 [PATCH] Findirect_function Manuel Giraud
  2022-01-14 12:09 ` Mattias Engdegård
@ 2022-01-14 12:16 ` Eli Zaretskii
  2022-01-14 12:29 ` Andreas Schwab
  2022-01-14 16:27 ` Stefan Monnier
  3 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2022-01-14 12:16 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Fri, 14 Jan 2022 11:40:02 +0100
> 
> Maybe it is bike shedding territory (and maybe I'm missing something)
> but here are two patches for Findirect_function and its usage. "make
> check" is the same and so far it works for me™.

Do we care about the additional cost a function call?  funcall
and apply are quite hot spots in Emacs, AFAIK.



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

* Re: [PATCH] Findirect_function
  2022-01-14 10:40 [PATCH] Findirect_function Manuel Giraud
  2022-01-14 12:09 ` Mattias Engdegård
  2022-01-14 12:16 ` Eli Zaretskii
@ 2022-01-14 12:29 ` Andreas Schwab
  2022-01-14 13:03   ` Manuel Giraud
  2022-01-14 16:27 ` Stefan Monnier
  3 siblings, 1 reply; 13+ messages in thread
From: Andreas Schwab @ 2022-01-14 12:29 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

On Jan 14 2022, Manuel Giraud wrote:

> diff --git a/src/data.c b/src/data.c
> index f287c38fcd..65babc49e9 100644
> --- a/src/data.c
> +++ b/src/data.c
> @@ -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;
>  }

Looks like a leftover from commit 60f8214e97.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



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

* Re: [PATCH] Findirect_function
  2022-01-14 12:09 ` Mattias Engdegård
@ 2022-01-14 13:00   ` Manuel Giraud
  0 siblings, 0 replies; 13+ messages in thread
From: Manuel Giraud @ 2022-01-14 13:00 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: emacs-devel

Mattias Engdegård <mattiase@acm.org> writes:

> 14 jan. 2022 kl. 11.40 skrev Manuel Giraud <manuel@ledu-giraud.fr>:
>
>> Maybe it is bike shedding territory (and maybe I'm missing something)
>
> Not at all, patches are always welcome!

Thanks.

[...]

> Good! See if you can find more instances of this unfortunate
> pattern. Even if it makes no difference for the generated code (very
> likely), it obfuscates the source code.

Ok, I'll look for this kind of pattern.

> The other changes are less obvious since they may introduce extra
> costs; some of these code paths are fairly hot. In addition, I'm
> giving part of the interpreter an overhaul so perhaps we could drop
> them for the time being. Do you agree?

Yes it is one more C function call. Sure you could drop it!
-- 
Manuel Giraud



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

* Re: [PATCH] Findirect_function
  2022-01-14 12:29 ` Andreas Schwab
@ 2022-01-14 13:03   ` Manuel Giraud
  0 siblings, 0 replies; 13+ messages in thread
From: Manuel Giraud @ 2022-01-14 13:03 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Jan 14 2022, Manuel Giraud wrote:
>
>> diff --git a/src/data.c b/src/data.c
>> index f287c38fcd..65babc49e9 100644
>> --- a/src/data.c
>> +++ b/src/data.c
>> @@ -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;
>>  }
>
> Looks like a leftover from commit 60f8214e97.

I see it is when Findirect_function could not signal an error
anymore. While here, could we remove the 'noerror' argument?
-- 
Manuel Giraud



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

* Re: [PATCH] Findirect_function
  2022-01-14 10:40 [PATCH] Findirect_function Manuel Giraud
                   ` (2 preceding siblings ...)
  2022-01-14 12:29 ` Andreas Schwab
@ 2022-01-14 16:27 ` Stefan Monnier
  2022-01-14 16:48   ` Manuel Giraud
  3 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2022-01-14 16:27 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

>    /* 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, Qt);
> +  if (NILP (fun))
> +    /* Let funcall get the error.  */
> +    fun = args[0];

This should remove the corresponding comment.
And I think I'd only be willing to make such a change if it comes with
some benchmarking showing that this optimization does not actually make
a difference.


        Stefan




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

* Re: [PATCH] Findirect_function
  2022-01-14 16:27 ` Stefan Monnier
@ 2022-01-14 16:48   ` Manuel Giraud
  2022-01-14 17:39     ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Manuel Giraud @ 2022-01-14 16:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>    /* 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, Qt);
>> +  if (NILP (fun))
>> +    /* Let funcall get the error.  */
>> +    fun = args[0];
>
> This should remove the corresponding comment.

You mean the /* Optimize… */ comment, right?

> And I think I'd only be willing to make such a change if it comes with
> some benchmarking showing that this optimization does not actually make
> a difference.

Do you happen to have some hints on how to do benchmarking in emacs? 😅
-- 
Manuel Giraud



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

* Re: [PATCH] Findirect_function
  2022-01-14 16:48   ` Manuel Giraud
@ 2022-01-14 17:39     ` Stefan Monnier
  2022-01-17 13:32       ` Manuel Giraud
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2022-01-14 17:39 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

>> This should remove the corresponding comment.
> You mean the /* Optimize… */ comment, right?

Yes.

>> And I think I'd only be willing to make such a change if it comes with
>> some benchmarking showing that this optimization does not actually make
>> a difference.
> Do you happen to have some hints on how to do benchmarking in emacs? 😅

There's the `elisp-benchmarks` package on GNU ELPA, and another standard
benchmark is to measure the time for Emacs to compile all its
`.el` files.

The `elisp-benchmarks` would welcome more benchmarks, so I'd encourage
you to write some of your own and contribute them there ;-)


        Stefan




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

* Re: [PATCH] Findirect_function
  2022-01-14 17:39     ` Stefan Monnier
@ 2022-01-17 13:32       ` Manuel Giraud
  2022-01-17 18:14         ` Stefan Monnier
  2022-01-17 18:16         ` Stefan Monnier
  0 siblings, 2 replies; 13+ messages in thread
From: Manuel Giraud @ 2022-01-17 13:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

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

* Re: [PATCH] Findirect_function
  2022-01-17 13:32       ` Manuel Giraud
@ 2022-01-17 18:14         ` Stefan Monnier
  2022-01-17 18:16         ` Stefan Monnier
  1 sibling, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2022-01-17 18:14 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

> It seems that we are in the same ballpark.

Indeed, it seems it's even faster!


        Stefan




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

* Re: [PATCH] Findirect_function
  2022-01-17 13:32       ` Manuel Giraud
  2022-01-17 18:14         ` Stefan Monnier
@ 2022-01-17 18:16         ` Stefan Monnier
  2022-01-18  0:43           ` Po Lu
  1 sibling, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2022-01-17 18:16 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: emacs-devel

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

Oh, the patch probably needs an entry etc/NEWS to say that the second
arg (which was declared obsolete in Emacs-25) was now removed.


        Stefan




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

* Re: [PATCH] Findirect_function
  2022-01-17 18:16         ` Stefan Monnier
@ 2022-01-18  0:43           ` Po Lu
  0 siblings, 0 replies; 13+ messages in thread
From: Po Lu @ 2022-01-18  0:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Manuel Giraud, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Oh, the patch probably needs an entry etc/NEWS to say that the second
> arg (which was declared obsolete in Emacs-25) was now removed.

Maybe not yet, since it was only obsoleted in Emacs 25.
Thanks.



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

end of thread, other threads:[~2022-01-18  0:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14 10:40 [PATCH] Findirect_function Manuel Giraud
2022-01-14 12:09 ` Mattias Engdegård
2022-01-14 13:00   ` Manuel Giraud
2022-01-14 12:16 ` Eli Zaretskii
2022-01-14 12:29 ` Andreas Schwab
2022-01-14 13:03   ` Manuel Giraud
2022-01-14 16:27 ` Stefan Monnier
2022-01-14 16:48   ` Manuel Giraud
2022-01-14 17:39     ` Stefan Monnier
2022-01-17 13:32       ` Manuel Giraud
2022-01-17 18:14         ` Stefan Monnier
2022-01-17 18:16         ` Stefan Monnier
2022-01-18  0:43           ` Po Lu

Code repositories for project(s) associated with this 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).