Skip to content

Consider re-enabling coderef-in-stash optimization #23131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jkeenan opened this issue Mar 18, 2025 · 1 comment
Open

Consider re-enabling coderef-in-stash optimization #23131

jkeenan opened this issue Mar 18, 2025 · 1 comment
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) defer-next-dev This PR should not be merged yet, but await the next development cycle

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Mar 18, 2025

During the 5.41 development cycle, b9eeeef was committed to blead, resulted in CPAN breakage late in the cycle and, at the request of the Perl Steering Committee (PSC) was reverted with an indication that we were open to further consideration in the 5.43 development cycle. We will use this ticket to track that discussion.

Original Commit

commit b9eeeef8c043fb0238a07e64636815bf327a6562
Author:     Lukas Mai <lukasmai.403@gmail.com>
AuthorDate: Fri Feb 14 21:49:03 2025 +0100
Commit:     Lukas Mai <lukasmai.403@gmail.com>
CommitDate: Sun Feb 16 01:42:36 2025 +0100

    op.c: re-enable coderef-in-stash optimization
    
    When seeing 'sub foo { ... }', perl does not need to allocate a full
    typeglob (with the subroutine being stored in the CODE slot). Instead,
    it can just store a coderef directly in the stash.
    
    This optimization was first announced in perl5220delta:
    
    > - Subroutines in packages no longer need to be stored in typeglobs:
    >   declaring a subroutine will now put a simple sub reference directly
    >   in the stash if possible, saving memory. The typeglob still
    >   notionally exists, so accessing it will cause the stash entry to be
    >   upgraded to a typeglob (i.e. this is just an internal implementation
    >   detail).  This optimization does not currently apply to XSUBs or
    >   exported subroutines, and method calls will undo it, since they
    >   cache things in typeglobs.  [GH #13392]
    
    However, due to a bug this optimization didn't actually work except for
    package main (GH #15671). The issue was fixed in v5.27.5, but the fix
    was backed out again in v5.27.9 because of CPAN breakage.
    
    This patch re-enables the optimization because I want to see what the
    current state of CPAN breakage is.

Resulting CPAN breakage (BBC tickets)

#23015
Log::Trace foundation POE::Component::Basement Prima Symbol::Get Test::Sims

#23055
Glib

#23065
Export::These

Some of the CPAN breakage was diagnosed as suboptimal code in the downstream modules and patches were submitted to those distribution's bug trackers.

PSC's request to revert
#23015 (comment)

Reversion performed: #23029

commit a634b79b81e660de6f05ab079e08d1a1aa1a8a07
Author:     Lukas Mai <lukasmai.403@gmail.com>
AuthorDate: Wed Feb 26 17:50:28 2025 +0100
Commit:     Graham Knop <haarg@haarg.org>
CommitDate: Mon Mar 3 05:22:59 2025 +0100

    Revert "op.c: re-enable coderef-in-stash optimization"
    
    This reverts commit b9eeeef8c043fb0238a07e64636815bf327a6562.
@jkeenan jkeenan added BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) defer-next-dev This PR should not be merged yet, but await the next development cycle labels Mar 18, 2025
@haarg
Copy link
Contributor

haarg commented Mar 18, 2025

I think we should re-enable this optimization early in the 5.43 cycle. Most of the issues have been fixed, and getting it in early should allow plenty of time to get any other fixes done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) defer-next-dev This PR should not be merged yet, but await the next development cycle
Projects
None yet
Development

No branches or pull requests

2 participants