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

Previous Next

Full log

View this message in rfc822 format

From: Lynn Winebarger <owinebar <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>, 79035 <at> debbugs.gnu.org
Subject: bug#79035: Remove micro-optimization for Funintern for performance
Date: Thu, 17 Jul 2025 09:32:25 -0400

[Message part 1 (text/plain, inline)]

On Thu, Jul 17, 2025, 2:44 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Lynn Winebarger <owinebar <at> gmail.com>
> > Date: Thu, 17 Jul 2025 01:24:03 -0400
> > Cc: 79035 <at> debbugs.gnu.org, Mattias Engdegård <
> mattias.engdegard <at> gmail.com>,
> >       Stefan Monnier <monnier <at> 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

here.
>

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

[Message part 2 (text/html, inline)]

Previous Next

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