On Thu, Jul 17, 2025, 1:04 AM Eli Zaretskii wrote: > > From: Lynn Winebarger > > 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