XS :reader & :writer accessors for core classes by richardleach · Pull Request #24187 · Perl/perl5

richardleach · 2026-02-11T00:28:45Z

Basic before/after benchmarks were run on a Linux VM:

use v5.42;
use Benchmark qw(cmpthese);
use experimental qw(class);

class Cor {
    field $name :param :reader :writer = "Perl";
}

my $x = Cor->new;

cmpthese(-3, {
    reader => sub { $x->name },
    writer => sub { $x->set_name("Perl") }
});

Crudely comparing the outputs for :reader:

Build	Throughput
Before	12655688/s
After	30834069/s

For :writer:

Build	Throughput
Before	9018387/s
After	21514669/s

This set of changes requires a perldelta entry, and I will write one if this PR goes ahead.

class.c

tonycoz · 2026-02-11T04:10:00Z

The other thing I considered was whether the XSUBs should support RC_STACK instead of depending on Perl_xs_wrap, but I only see a test case for that (rc_add in XS::APItest).

richardleach · 2026-02-11T23:12:45Z

The other thing I considered was whether the XSUBs should support RC_STACK instead of depending on Perl_xs_wrap, but I only see a test case for that (rc_add in XS::APItest).

I wasn't sure about that either. Couldn't remember - and haven't yet checked - if all XSUBs will get the split-stack treatement or what.

tonycoz · 2026-02-11T23:42:19Z

I wasn't sure about that either. Couldn't remember - and haven't yet checked - if all XSUBs will get the split-stack treatement or what.

You can set CvXS_RCSTACK() on the XSUB CV if it is RCSTACK safe, which the current implementations aren't.

richardleach · 2026-02-13T23:22:15Z

I wasn't sure about that either. Couldn't remember - and haven't yet checked - if all XSUBs will get the split-stack treatement or what.

You can set CvXS_RCSTACK() on the XSUB CV if it is RCSTACK safe, which the current implementations aren't.

Hopefully they are now. 😁

leonerd

I see this is still draft, but while I'm here here's a few comments anyway.

class.c

The existing `:reader` and `:writer` accessors are replaced with XS CVs to more than double performance (in the simple scalar field case, at least).

tonycoz · 2026-02-16T22:46:19Z

class.c

+
+    SV* const fsv = fields[fieldix];
+    /* Assigning magic to a field SV is currently unused and unsupported. */
+    assert(!SvMAGICAL(fsv));


Magic is everywhere, if you know where to look:

$ ./perl -Ilib -MDevel::Peek -Mfeature=class -e 'class C { field $x:reader :writer; method f { length $x } method d { ::Dump($x) } } my $c = C->new; $c->set_x("\x{100}"); $c->f; $c->d; $c->x' class is experimental at -e line 1. field is experimental at -e line 1. method is experimental at -e line 1. method is experimental at -e line 1. SV = PVMG(0x561a15549be8) at 0x561a154d3900 REFCNT = 2 FLAGS = (SMG,POK,IsCOW,pPOK,UTF8) IV = 0 NV = 0 PV = 0x561a15530348 "\xC4\x80"\0 [UTF8 "\x{100}"] CUR = 2 LEN = 16 COW_REFCNT = 1 MAGIC = 0x561a15518dc8 MG_VIRTUAL = &PL_vtbl_utf8 MG_TYPE = PERL_MAGIC_utf8(w) MG_LEN = 1 perl: class.c:396: class_reader_sv_xsub: Assertion `!SvMAGICAL(fsv)' failed. Aborted

And as I said elsewhere, I don't think it's reasonable to restrict ties from fields anyway.

Another possible hole in the "no magic" is refaliasing, field %env; ... \%env = \%ENV

Though there seems to be another bug here:

$ gdb --args ./perl -Ilib -MDevel::Peek -Mfeature=class,refaliasing -le 'class C { field %x:reader; method d { ::Dump(%x) } method add_magic_blue_smoke { \%x = \%ENV; $self } } my $c = C->new->add_magic_blue_smoke; print $c->x' ... Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". class is experimental at -e line 1. field is experimental at -e line 1. method is experimental at -e line 1. method is experimental at -e line 1. Aliasing via reference is experimental at -e line 1. perl: class.c:481: class_reader_hv_xsub: Assertion `SvTYPE(fsv) == SVt_PVHV' failed. Program received signal SIGABRT, Aborted. __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 warning: 44 ./nptl/pthread_kill.c: No such file or directory (gdb) up 5 #5 0x0000555555629451 in class_reader_hv_xsub (my_perl=0x555555cbd2a0, cv=0x555555d53588) at class.c:481 481 assert(SvTYPE(fsv) == SVt_PVHV); (gdb) call Perl_sv_dump(my_perl, fsv) SV = UNKNOWN(0xff) (0x555555ce3870) at 0x555555cc0918 REFCNT = 0 FLAGS = ()

(non-RC_STACK)

But that crashes without this patch, yay.

tonycoz · 2026-02-16T22:48:53Z

class.c

+    }
+
+    dXSTARG;
+    SV *rsv = SvROK(fsv) ? sv_newmortal() : TARG;


If we do allow ties on fields, this should SvGETMAGIC() before checking SvROK().

tonycoz · 2026-02-16T23:24:48Z

class.c

+        SV* cvname = newSV_type_mortal(SVt_PV);
+        gv_fullname4(cvname, gv, NULL, TRUE);
+        croak("Too many arguments for subroutine '%" SVf "' (got %" UVuf "; expected %" UVuf ")",
+            cvname, items, expected);


items is never a UV, it is either I32 or SSize_t.

tonycoz · 2026-02-16T23:38:41Z

class.c

+
+    SV* const fsv = fields[fieldix];
+    /* Assigning magic to a field SV is currently unused and unsupported. */
+    assert(!SvMAGICAL(fsv));


Similar problem to the above:

$ ./perl -Ilib -MDevel::Peek -Mfeature=class -e 'class C { field $x:reader :writer; method f { length $x } method d { ::Dump($x) } } my $c = C->new; $c->set_x("\x{100}"); $c->f; $c->d; $c->set_x(1)' class is experimental at -e line 1. field is experimental at -e line 1. method is experimental at -e line 1. method is experimental at -e line 1. SV = PVMG(0x55654e1dba28) at 0x55654e165900 REFCNT = 3 FLAGS = (SMG,POK,IsCOW,pPOK,UTF8) IV = 0 NV = 0 PV = 0x55654e1c1c18 "\xC4\x80"\0 [UTF8 "\x{100}"] CUR = 2 LEN = 16 COW_REFCNT = 1 MAGIC = 0x55654e1aa758 MG_VIRTUAL = &PL_vtbl_utf8 MG_TYPE = PERL_MAGIC_utf8(w) MG_LEN = 1 perl: class.c:565: class_writer_sv_xsub: Assertion `!SvMAGICAL(fsv)' failed. Aborted

tonycoz · 2026-02-16T23:46:07Z

class.c

+        }
+        case G_LIST:
+        {
+            rpp_popfree_to(PL_stack_sp - 1);


For RCSTACK, what if this was the last reference?

richardleach marked this pull request as draft February 11, 2026 00:28

tonycoz reviewed Feb 11, 2026

View reviewed changes

class.c Outdated Show resolved Hide resolved

tonycoz reviewed Feb 11, 2026

View reviewed changes

class.c Outdated Show resolved Hide resolved

tonycoz reviewed Feb 11, 2026

View reviewed changes

class.c Outdated Show resolved Hide resolved

richardleach force-pushed the perlclass_cv_accessors branch from f1d6cd9 to 6d4eeb8 Compare February 11, 2026 23:10

richardleach force-pushed the perlclass_cv_accessors branch from 6d4eeb8 to 5b7b053 Compare February 13, 2026 23:00

richardleach requested a review from leonerd February 13, 2026 23:28

richardleach changed the title ~~XS Accessors for core Classes - WIP~~ XS :reader & :writer accessors for core classes Feb 13, 2026

richardleach mentioned this pull request Feb 13, 2026

Faster path for (simple) class :reader and :writer accessors #24082

Draft

leonerd reviewed Feb 14, 2026

View reviewed changes

richardleach force-pushed the perlclass_cv_accessors branch from 5b7b053 to 01eae7a Compare February 15, 2026 13:53

richardleach marked this pull request as ready for review February 15, 2026 13:53

richardleach force-pushed the perlclass_cv_accessors branch from 01eae7a to 6a09fa4 Compare February 15, 2026 14:07

richardleach added 2 commits February 15, 2026 14:11

XS :reader & :writer accessors for core classes

8454d78

The existing `:reader` and `:writer` accessors are replaced with XS CVs to more than double performance (in the simple scalar field case, at least).

Perldelta for GH#24187 (XS accessors)

63c9862

richardleach force-pushed the perlclass_cv_accessors branch from 6a09fa4 to 63c9862 Compare February 15, 2026 14:12

tonycoz reviewed Feb 16, 2026

View reviewed changes

class.c

}

case G_LIST:

{

rpp_popfree_to(PL_stack_sp - 1);

Copy link

Contributor

tonycoz Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For RCSTACK, what if this was the last reference?

Conversation

richardleach commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonycoz commented Feb 11, 2026

Uh oh!

richardleach commented Feb 11, 2026

Uh oh!

tonycoz commented Feb 11, 2026

Uh oh!

richardleach commented Feb 13, 2026

Uh oh!

leonerd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonycoz Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonycoz Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

tonycoz Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

tonycoz Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

tonycoz Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

tonycoz Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

richardleach commented Feb 11, 2026 •

edited

Loading

tonycoz Feb 16, 2026 •

edited

Loading