On Thu, Jul 17, 2025, 2:44 AM Eli Zaretskii <
eliz@gnu.org> wrote:
> From: Lynn Winebarger <owinebar@gmail.com>
> Date: Thu, 17 Jul 2025 01:24:03 -0400
> Cc: 79035@debbugs.gnu.org, Mattias Engdegård <mattias.engdegard@gmail.com>,
> Stefan Monnier <monnier@iro.umontreal.ca>
>
> > 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.
If this change is related to some other changeset, let alone is needed
by it, it is not standalone, and should be part of that other
changeset. It would make sense to have it as standalone if it brought
us some significant advantages, which doesn't seem to be the case
It's related in the sense that it's removing static variables in lread.c. But this patch should probably be considered cleanup from the introduction of shorthands. I think the change is actually trivial for someone familiar with lread.c.
At any rate, the feedback on the patch for making read and friends reentrant was that it was too much at once. I'm guessing Mattias and/or Stefan would prefer to judge this change independently of the rest, but I can wait for their feedback.
Lynn