GNU bug report logs -
#79035
Remove micro-optimization for Funintern for performance
Previous Next
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
On Thu, Jul 17, 2025, 1:04 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > 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.
>
The performance improvement is the punchline. The purpose of the change is
to remove a static variable. That's why I originally included it in the
patch to make the code in lread.c re-entrant. But it probably makes more
sense as a stand-alone change.
Lynn
[Message part 2 (text/html, inline)]
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.