Change 31770 by [EMAIL PROTECTED] on 2007/08/31 09:07:51
Subject: Re: optimize push @ISA, (was Re: parent.pm at
http://corion.net/perl-dev)
From: "Brandon Black" <[EMAIL PROTECTED]>
Date: Sun, 12 Aug 2007 13:36:14 -0700
Message-ID: <[EMAIL PROTECTED]>
Affected files ...
... //depot/perl/av.c#120 edit
... //depot/perl/embedvar.h#255 edit
... //depot/perl/gv.h#74 edit
... //depot/perl/intrpvar.h#215 edit
... //depot/perl/lib/mro.pm#7 edit
... //depot/perl/mg.c#499 edit
... //depot/perl/perlapi.h#177 edit
... //depot/perl/pp.c#597 edit
... //depot/perl/pp_hot.c#524 edit
... //depot/perl/sv.c#1420 edit
Differences ...
==== //depot/perl/av.c#120 (text) ====
Index: perl/av.c
--- perl/av.c#119~31473~ 2007-06-26 09:12:27.000000000 -0700
+++ perl/av.c 2007-08-31 02:07:51.000000000 -0700
@@ -342,11 +342,14 @@
SvREFCNT_dec(ary[key]);
ary[key] = val;
if (SvSMAGICAL(av)) {
+ const MAGIC* const mg = SvMAGIC(av);
if (val != &PL_sv_undef) {
- const MAGIC* const mg = SvMAGIC(av);
sv_magic(val, (SV*)av, toLOWER(mg->mg_type), 0, key);
}
- mg_set((SV*)av);
+ if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ PL_delaymagic |= DM_ARRAY;
+ else
+ mg_set((SV*)av);
}
return &ary[key];
}
@@ -428,8 +431,13 @@
Perl_croak(aTHX_ PL_no_modify);
/* Give any tie a chance to cleanup first */
- if (SvRMAGICAL(av))
- mg_clear((SV*)av);
+ if (SvRMAGICAL(av)) {
+ const MAGIC* const mg = SvMAGIC(av);
+ if (PL_delaymagic && mg->mg_type == PERL_MAGIC_isa)
+ PL_delaymagic |= DM_ARRAY;
+ else
+ mg_clear((SV*)av);
+ }
if (AvMAX(av) < 0)
return;
==== //depot/perl/embedvar.h#255 (text+w) ====
Index: perl/embedvar.h
--- perl/embedvar.h#254~31280~ 2007-05-25 16:26:33.000000000 -0700
+++ perl/embedvar.h 2007-08-31 02:07:51.000000000 -0700
@@ -116,7 +116,6 @@
#define PL_defgv (vTHX->Idefgv)
#define PL_defoutgv (vTHX->Idefoutgv)
#define PL_defstash (vTHX->Idefstash)
-#define PL_delayedisa (vTHX->Idelayedisa)
#define PL_delaymagic (vTHX->Idelaymagic)
#define PL_diehook (vTHX->Idiehook)
#define PL_dirty (vTHX->Idirty)
@@ -431,7 +430,6 @@
#define PL_Idefgv PL_defgv
#define PL_Idefoutgv PL_defoutgv
#define PL_Idefstash PL_defstash
-#define PL_Idelayedisa PL_delayedisa
#define PL_Idelaymagic PL_delaymagic
#define PL_Idiehook PL_diehook
#define PL_Idirty PL_dirty
==== //depot/perl/gv.h#74 (text) ====
Index: perl/gv.h
--- perl/gv.h#73~31396~ 2007-06-15 23:10:19.000000000 -0700
+++ perl/gv.h 2007-08-31 02:07:51.000000000 -0700
@@ -181,6 +181,7 @@
#define DM_UID 0x003
#define DM_RUID 0x001
#define DM_EUID 0x002
+#define DM_ARRAY 0x004
#define DM_GID 0x030
#define DM_RGID 0x010
#define DM_EGID 0x020
==== //depot/perl/intrpvar.h#215 (text) ====
Index: perl/intrpvar.h
--- perl/intrpvar.h#214~31280~ 2007-05-25 16:26:33.000000000 -0700
+++ perl/intrpvar.h 2007-08-31 02:07:51.000000000 -0700
@@ -180,8 +180,6 @@
PERLVARI(Iregmatch_slab, regmatch_slab *, NULL)
PERLVAR(Iregmatch_state, regmatch_state *)
-PERLVARI(Idelayedisa, HV*, NULL) /* stash for PL_delaymagic for
magic_setisa */
-
/* Put anything new that is pointer aligned here. */
PERLVAR(Idelaymagic, U16) /* ($<,$>) = ... */
==== //depot/perl/lib/mro.pm#7 (text) ====
Index: perl/lib/mro.pm
--- perl/lib/mro.pm#6~31332~ 2007-06-04 01:04:13.000000000 -0700
+++ perl/lib/mro.pm 2007-08-31 02:07:51.000000000 -0700
@@ -319,8 +319,8 @@
Specifying the mro type of a class before setting C<@ISA> will
be faster than the other way around. Also, making all of your
-C<@ISA> manipulations in a single assignment statement will be
-faster that doing them one by one via C<push> (which is what
+C<@ISA> manipulations in a single assignment or push statement
+will be faster that doing them one by one (which is what
C<use base> does currently).
Examples:
@@ -330,23 +330,29 @@
use base qw/A B C/;
use mro 'c3';
+ # Equivalently slow
+ package Foo;
+ our @ISA;
+ require A; push(@ISA, 'A');
+ require B; push(@ISA, 'B');
+ require C; push(@ISA, 'C');
+ use mro 'c3';
+
# The fastest way
# (not exactly equivalent to above,
# as base.pm can do other magic)
+ package Foo;
use mro 'c3';
- use A ();
- use B ();
- use C ();
+ require A;
+ require B;
+ require C;
our @ISA = qw/A B C/;
Generally speaking, every time C<@ISA> is modified, the MRO
-of that class will be recalculated, because of the way array
-magic works. Pushing multiple items onto C<@ISA> in one push
-statement still counts as multiple modifications. However,
-assigning a list to C<@ISA> only counts as a single
-modification. Thus if you really need to do C<push> as
-opposed to assignment, C<@ISA = (@ISA, qw/A B C/);>
-will still be faster than C<push(@ISA, qw/A B C/);>
+of that class will be recalculated because of the way array
+magic works. Cutting down on unecessary recalculations is
+a win, especially with complex class hierarchies and/or
+the c3 mro.
=head1 SEE ALSO
==== //depot/perl/mg.c#499 (text) ====
Index: perl/mg.c
--- perl/mg.c#498~31765~ 2007-08-30 06:49:14.000000000 -0700
+++ perl/mg.c 2007-08-31 02:07:51.000000000 -0700
@@ -1528,6 +1528,10 @@
/* Bail out if destruction is going on */
if(PL_dirty) return 0;
+ /* Skip _isaelem because _isa will handle it shortly */
+ if (PL_delaymagic & DM_ARRAY && mg->mg_type == PERL_MAGIC_isaelem)
+ return 0;
+
/* XXX Once it's possible, we need to
detect that our @ISA is aliased in
other stashes, and act on the stashes
@@ -1542,10 +1546,7 @@
: (GV*)SvMAGIC(mg->mg_obj)->mg_obj
);
- if(PL_delaymagic)
- PL_delayedisa = stash;
- else
- mro_isa_changed_in(stash);
+ mro_isa_changed_in(stash);
return 0;
}
==== //depot/perl/perlapi.h#177 (text+w) ====
Index: perl/perlapi.h
--- perl/perlapi.h#176~31280~ 2007-05-25 16:26:33.000000000 -0700
+++ perl/perlapi.h 2007-08-31 02:07:51.000000000 -0700
@@ -268,8 +268,6 @@
#define PL_defoutgv (*Perl_Idefoutgv_ptr(aTHX))
#undef PL_defstash
#define PL_defstash (*Perl_Idefstash_ptr(aTHX))
-#undef PL_delayedisa
-#define PL_delayedisa (*Perl_Idelayedisa_ptr(aTHX))
#undef PL_delaymagic
#define PL_delaymagic (*Perl_Idelaymagic_ptr(aTHX))
#undef PL_diehook
==== //depot/perl/pp.c#597 (text) ====
Index: perl/pp.c
--- perl/pp.c#596~31765~ 2007-08-30 06:49:14.000000000 -0700
+++ perl/pp.c 2007-08-31 02:07:51.000000000 -0700
@@ -4420,12 +4420,17 @@
PUSHi( AvFILL(ary) + 1 );
}
else {
+ PL_delaymagic = DM_DELAY;
for (++MARK; MARK <= SP; MARK++) {
SV * const sv = newSV(0);
if (*MARK)
sv_setsv(sv, *MARK);
av_store(ary, AvFILLp(ary)+1, sv);
}
+ if (PL_delaymagic & DM_ARRAY)
+ mg_set((SV*)ary);
+
+ PL_delaymagic = 0;
SP = ORIGMARK;
PUSHi( AvFILLp(ary) + 1 );
}
==== //depot/perl/pp_hot.c#524 (text) ====
Index: perl/pp_hot.c
--- perl/pp_hot.c#523~31508~ 2007-06-30 09:19:33.000000000 -0700
+++ perl/pp_hot.c 2007-08-31 02:07:51.000000000 -0700
@@ -1122,6 +1122,9 @@
PL_egid = PerlProc_getegid();
}
PL_tainting |= (PL_uid && (PL_euid != PL_uid || PL_egid != PL_gid));
+
+ if (PL_delaymagic & DM_ARRAY && SvMAGICAL((SV*)ary))
+ mg_set((SV*)ary);
}
PL_delaymagic = 0;
@@ -1152,14 +1155,6 @@
*relem++ = (lelem <= lastlelem) ? *lelem++ : &PL_sv_undef;
}
- /* This is done at the bottom and in this order because
- mro_isa_changed_in() can throw exceptions */
- if(PL_delayedisa) {
- HV* stash = PL_delayedisa;
- PL_delayedisa = NULL;
- mro_isa_changed_in(stash);
- }
-
RETURN;
}
==== //depot/perl/sv.c#1420 (text) ====
Index: perl/sv.c
--- perl/sv.c#1419~31765~ 2007-08-30 06:49:14.000000000 -0700
+++ perl/sv.c 2007-08-31 02:07:51.000000000 -0700
@@ -11167,7 +11167,6 @@
PL_sub_generation = proto_perl->Isub_generation;
PL_isarev = hv_dup_inc(proto_perl->Iisarev, param);
- PL_delayedisa = hv_dup_inc(proto_perl->Idelayedisa, param);
/* funky return mechanisms */
PL_forkprocess = proto_perl->Iforkprocess;
End of Patch.
