GNU bug report logs - #79035
Remove micro-optimization for Funintern for performance

Previous Next

Package: emacs;

Reported by: Lynn Winebarger <owinebar <at> gmail.com>

Date: Thu, 17 Jul 2025 03:26:02 UTC

Severity: normal

Full log


Message #8 received at 79035 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lynn Winebarger <owinebar <at> gmail.com>
Cc: mattias.engdegard <at> gmail.com, monnier <at> iro.umontreal.ca,
 79035 <at> debbugs.gnu.org
Subject: Re: bug#79035: Remove micro-optimization for Funintern for performance
Date: Thu, 17 Jul 2025 08:04:47 +0300
> From: Lynn Winebarger <owinebar <at> gmail.com>
> 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.

That's just 0.5% of speedup, which doesn't pass my threshold for
worthy changes, especially when the code changes are non-trivial (as
they are in this case).  So my vote is against installing this, sorry.

However, I'm open to other opinions.




This bug report was last modified 10 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.