Subject: bug#79035: Remove micro-optimization for Funintern for performance

From: Lynn Winebarger <[email-masked]>
Date: Wed, 16 Jul 2025 23:25:15 -0400

Based on my measurement of my reentrant reader submission, I tried
just eliminating the static variable used to "optimize" out a
redundant lookup by stashing the index in a static variable used only
by Funintern.  See attached - I just repeated the lookup code in
Funintern, removed the assignment from oblookup, and eliminated the
static variable.  At least for -O0, storing this information in the
frequently called oblookup to save a lookup in the very infrequently
called Funintern is a bit of a pessimization.  Based on summing the
profiles of all the byte-compile jobs, and filtering for
obarray-related functions in read.c, I get the following for commit
a8b65860a5e28ee0867e8506a17d74d4a9b7783a (the cumulative is all
functions in lread.c, most of which I am eliding):
Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total
 time   seconds   seconds    calls   s/call   s/call  name
 13.44     31.82     8.68 79508607     0.00     0.00  oblookup
  0.85     58.97     0.55      260     2.12     2.21  grow_obarray
  0.84     59.51     0.54 83352123     0.00     0.00  obarray_index
  0.81     60.03     0.52 78691233     0.00     0.00
oblookup_considering_shorthand
  0.37     62.19     0.24  6858863     0.00     0.00  intern_sym
  0.29     63.03     0.19  4635384     0.00     0.00  read_internal_start
  0.08     64.05     0.05 20570557     0.00     0.00  mapatoms_1
  0.06     64.23     0.04     1060     0.04     0.04  map_obarray
  0.05     64.30     0.03  6858863     0.00     0.00  intern_driver
  0.05     64.33     0.03  1143939     0.00     0.00  Fintern
  0.03     64.48     0.02                             Funintern
  0.02     64.57     0.01    66751     0.00     0.00  Fintern_soft
  0.00     64.59     0.00    22626     0.00     0.00
Flread__substitute_object_in_subtree
  0.00     64.59     0.00    22626     0.00     0.00  substitute_object_recurse
  0.00     64.59     0.00    13223     0.00     0.00  Fobarray_make
  0.00     64.59     0.00    13223     0.00     0.00  allocate_obarray
  0.00     64.59     0.00    13223     0.00     0.00  make_obarray
  0.00     64.59     0.00    10916     0.00     0.00  Fobarrayp
  0.00     64.59     0.00     5032     0.00     0.00  Flocate_file_internal
  0.00     64.59     0.00     2522     0.00     0.00  intern_1
  0.00     64.59     0.00     1043     0.00     0.00  Fmapatoms

The same commit with the attached patch:
 13.22     31.13     8.23 79563723     0.00     0.00  oblookup
  0.67     57.22     0.42      264     1.59     1.66  grow_obarray
  0.61     57.60     0.38 83407339     0.00     0.00  obarray_index
  0.61     57.98     0.38 78745509     0.00     0.00
oblookup_considering_shorthand
  0.25     60.64     0.16  6858324     0.00     0.00  intern_sym
  0.19     61.32     0.12  4635494     0.00     0.00  read_internal_start
  0.11     61.59     0.07 20669351     0.00     0.00  mapatoms_1
  0.11     61.66     0.07     1067     0.07     0.07  map_obarray
  0.10     61.84     0.06                             Funintern
  0.05     62.10     0.03    66913     0.00     0.00  Fintern_soft
  0.01     62.27     0.01  6858324     0.00     0.00  intern_driver
  0.00     62.28     0.00  1146143     0.00     0.00  Fintern
  0.00     62.28     0.00    22460     0.00     0.00
Flread__substitute_object_in_subtree
  0.00     62.28     0.00    22460     0.00     0.00  substitute_object_recurse
  0.00     62.28     0.00    13219     0.00     0.00  Fobarray_make
  0.00     62.28     0.00    13219     0.00     0.00  allocate_obarray
  0.00     62.28     0.00    13219     0.00     0.00  make_obarray
  0.00     62.28     0.00    10918     0.00     0.00  Fobarrayp
  0.00     62.28     0.00     5031     0.00     0.00  Flocate_file_internal
  0.00     62.28     0.00     2522     0.00     0.00  intern_1
  0.00     62.28     0.00     1050     0.00     0.00  Fmapatoms

The total reported cpu time for
a8b65860a5e28ee0867e8506a17d74d4a9b7783a is 3479.51s, and with the
patch it is 3460.69.
You can see Funintern is more expensive, but also that in all those
jobs no actual calls were recorded in either case.  I see similar
differences for these functions when I compare results for commit
2bdcf0250acecdb0719203ae711aedf5baad1783.

As a reminder, I submitted my assignment paperwork a couple of years
ago, although this seems like a pretty trivial change.
Lynn
From c4818ba01f94717876f772f9a0b5e67d2d32d532 Mon Sep 17 00:00:00 2001
From: Onnie Lynn Winebarger <[email-masked]>
Date: Wed, 16 Jul 2025 23:04:07 -0400
Subject: [PATCH] Remove micro-optimization for Funintern

(lread.c): Remove static variable oblookup_last_bucket_number.  Removal
appears to reduce execution time of obarray functions by about 6%, while
the variable was only used to avoid a redundant lookup in the rarely
called Funintern.
---
 src/lread.c | 53 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 5b7848c3118..ad25fe052a1 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4927,10 +4927,6 @@ string_to_number (char const *string, int base, ptrdiff_t *plen)
 
 static Lisp_Object initial_obarray;
 
-/* `oblookup' stores the bucket number here, for the sake of Funintern.  */
-
-static size_t oblookup_last_bucket_number;
-
 static Lisp_Object make_obarray (unsigned bits);
 
 /* Slow path obarray check: return the obarray to use or signal an error.  */
@@ -5141,6 +5137,14 @@ DEFUN ("intern-soft", Fintern_soft, Sintern_soft, 1, 2, 0,
     }
 }
 
+/* Bucket index of the string STR of length SIZE_BYTE bytes in obarray OA.  */
+static ptrdiff_t
+obarray_index (struct Lisp_Obarray *oa, const char *str, ptrdiff_t size_byte)
+{
+  EMACS_UINT hash = hash_char_array (str, size_byte);
+  return knuth_hash (reduce_emacs_uint_to_hash_hash (hash), oa->size_bits);
+}
+
 DEFUN ("unintern", Funintern, Sunintern, 2, 2, 0,
        doc: /* Delete the symbol named NAME, if any, from OBARRAY.
 The value is t if a symbol was found and deleted, nil otherwise.
@@ -5174,14 +5178,15 @@ DEFUN ("unintern", Funintern, Sunintern, 2, 2, 0,
 					SCHARS (string), SBYTES (string),
 					&longhand, &longhand_chars,
 					&longhand_bytes);
-  if (longhand)
-    xfree(longhand);
 
-  if (FIXNUMP (tem))
-    return Qnil;
+  if (FIXNUMP (tem)
   /* If arg was a symbol, don't delete anything but that symbol itself.  */
-  if (BARE_SYMBOL_P (name) && !BASE_EQ (name, tem))
-    return Qnil;
+      || (BARE_SYMBOL_P (name) && !BASE_EQ (name, tem)))
+    {
+      if (longhand)
+	xfree (longhand);
+      return Qnil;
+    }
 
   /* There are plenty of other symbols which will screw up the Emacs
      session if we unintern them, as well as even more ways to use
@@ -5193,7 +5198,24 @@ DEFUN ("unintern", Funintern, Sunintern, 2, 2, 0,
   struct Lisp_Symbol *sym = XBARE_SYMBOL (tem);
   sym->u.s.interned = SYMBOL_UNINTERNED;
 
-  ptrdiff_t idx = oblookup_last_bucket_number;
+  ptrdiff_t idx = -1;
+  {
+    /* Removed static variable oblookup_last_bucket_number micro-optimization
+       Don't use a static variable just to avoid this
+       - how frequently used is unintern that using a static variable
+       to optimize away this redundant index calculation would be
+       worth adding a second reference parameter? */
+    struct Lisp_Obarray *o = XOBARRAY (obarray);
+    if (longhand)
+      {
+	idx = obarray_index (o, longhand, longhand_bytes);
+	xfree (longhand);
+      }
+    else
+      {
+	idx = obarray_index (o,SSDATA (string), SBYTES (string));
+      }
+  }
   Lisp_Object *loc = &XOBARRAY (obarray)->buckets[idx];
 
   eassert (BARE_SYMBOL_P (*loc));
@@ -5218,14 +5240,6 @@ DEFUN ("unintern", Funintern, Sunintern, 2, 2, 0,
 }
 
 
-/* Bucket index of the string STR of length SIZE_BYTE bytes in obarray OA.  */
-static ptrdiff_t
-obarray_index (struct Lisp_Obarray *oa, const char *str, ptrdiff_t size_byte)
-{
-  EMACS_UINT hash = hash_char_array (str, size_byte);
-  return knuth_hash (reduce_emacs_uint_to_hash_hash (hash), oa->size_bits);
-}
-
 /* Return the symbol in OBARRAY whose name matches the string
    of SIZE characters (SIZE_BYTE bytes) at PTR.
    If there is no such symbol, return the integer bucket number of
@@ -5240,7 +5254,6 @@ oblookup (Lisp_Object obarray, register const char *ptr, ptrdiff_t size, ptrdiff
   ptrdiff_t idx = obarray_index (o, ptr, size_byte);
   Lisp_Object bucket = o->buckets[idx];
 
-  oblookup_last_bucket_number = idx;
   if (!BASE_EQ (bucket, make_fixnum (0)))
     {
       Lisp_Object sym = bucket;
-- 
2.50.0