Still "keys" performance regression between 5.10.1 and 5.12.3-RC1 #11028
Comments
From btik-fbsd@scoubidou.comCreated by btik-fbsd@scoubidou.comThere is a performance regression since 5.10.1 when using core keys my $hash = { 1 .. 1000 }; my $count = -10; On the same host, FreeBSD 8.1-RELEASE amd64: * with perl 5.10.1 (non threaded perl, FreeBSD ports version) * with last perl 5.12.3-RC1 Perl Info |
From btik-fbsd@scoubidou.comNote that this problem affects 5.12.3-RC1, not 5.10.1. I did the benchmark without installing 5.12.3, and called perlbug as Sorry... |
|
btik-fbsd@scoubidou.com - Status changed from 'new' to 'open' |
From btik-fbsd@scoubidou.comJust created a new report specific to 5.12.3-RC1. Please, forget this one and sorry for the noise. |
From btik-fbsd@scoubidou.comThis is a bug report for perl from btik-fbsd@scoubidou.com, There is a performance regression since 5.10.1 when using core keys use strict; my $hash = { 1 .. 1000 }; my $count = -10; On the same host, FreeBSD 8.1-RELEASE amd64: * with perl 5.10.1 (non threaded perl, FreeBSD ports version) * with last perl 5.12.3-RC1 Max. Flags: Site configuration information for perl 5.12.3: Configured by max at Tue Jan 11 22:58:26 CET 2011. Summary of my perl5 (revision 5 version 12 subversion 3) configuration: Locally applied patches: @INC for perl 5.12.3: Environment for perl 5.12.3: |
From @nwc10On Tue, Jan 11, 2011 at 03:13:40PM -0800, btik-fbsd @ scoubidou. com wrote:
There is a performance regression, but it's not directly to do with If I take your example:
I can replicate it: $ cat 82100.pl my $hash = { 1 .. 1000 }; my $count = -10; If I switch from lexical to package variables: $ cat 82100p.pl my $hash = { 1 .. 1000 }; my $count = -10; there's no change. So it can't directly be anything to do with the use of I believe that the speed change is actually due to the removal of an I'm not sure what can be done about that. The optimisation code for list Nicholas Clark |
|
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Tue, Jan 11, 2011 at 09:21:32AM -0800, btik-fbsd@scoubidou.com wrote:
This is due to the COMMON-flag pessimisation of the array assignment op 5.10.0 and *doesn't* get set in 5.8.x so 5.10.1 (and 5.8.x) are faster. Now, I remember there was a bug related to the COMMON flag on 5.10.0 that Anyway, its not a regression from any earlier 5.12.x, so isn't a blocker -- |
From @cpansproutOn Wed Jan 12 06:44:31 2011, davem wrote:
by change fafafba, which caused bug #70171.
Probably because #70171 was fixed by 0f907b9, which got into 5.12. An expression like ‘my @array = ...’ might also reference @array on the If the RHS has a hash dereference, keys %hash or similar, we could
|
From @iabynOn Sun, Jan 16, 2011 at 01:28:11PM -0800, Father Chrysostomos via RT wrote:
But we should be able to tell whether the lexical is closed over by its $ p -MDevel::Peek -we'my $x; Dump $x' $ p -MDevel::Peek -we'my $x; Dump $x; sub f {$x+1}' -- |
From @cpansproutOn Tue Jan 18 06:53:20 2011, davem wrote:
At compile time, the OPpASSIGN_COMMON flag is set when the list |
From @iabynOn Sun, Feb 06, 2011 at 01:07:25PM -0800, Father Chrysostomos via RT wrote:
And thinking further, you can get this behaviour without a closure too: my $r; So I think this assignment is correctly pessimised, and shouldn't I'll close the ticket. -- |
|
@iabyn - Status changed from 'open' to 'rejected' |
From @demerphqOn 10 March 2011 18:04, Dave Mitchell <davem@iabyn.com> wrote:
Am I right in understanding that our position is that a 30% slowdown my $r; work correctly and not result in "semi-panic: attempt to dup freed I personally do not think that this is so clear cut, given that they my $r; And have it work correctly? I have not fully understood FC's closure case properly, so maybe this IOW, I'm not sure that sacrificing performance on a very common While I don't think id go so far as to say we should actually choosing cheers, -- |
From @iabynOn Sat, Mar 12, 2011 at 01:51:46PM +0100, demerphq wrote:
Note its only these two my @array = @$arrayref; that get the pessimisation, and not the more general versions of my @array = something;
I'm not sure what point you're trying to make here. In that code, the
I don't understand what you mean by a workaround.
We're not talking speed over correctness here. We're talking speed over -- |
From @lizmatOn Mar 12, 2011, at 9:58 PM, Dave Mitchell wrote:
FWIW, in the cases where I've used this construct, it was for creating local copies of a parameter hash. On the one hand, that would always be small hashes. If such a subroutine gets called a lot, it would probably make sense to change the input parameters anyway. Since there is almost no performance difference between $foo[...] / $foo{...} and $foo->[...] / $foo->{...}, I don't see a point why anyone would need to make copies just for that reason. If there are other reasons you need a copy, then I guess you will have to pay for that.
Good to know. :-) Liz |
From @demerphqOn 12 March 2011 21:58, Dave Mitchell <davem@iabyn.com> wrote:
So then this doesn't explain: my @array= keys %$hash; being slower? Isn't that what this thread/bug is about?
By workaround I meant that moving the my one line up "makes it work ok".
Yes, the panic is why I was being so mealy-mouthed about things. Thanks for explaining things. cheers, -- |
From @iabynOn Sat, Mar 12, 2011 at 10:17:51PM +0100, demerphq wrote:
The important point is the hash deref on the RHS. However, this is all happily now irrelevant, with this commit I've just commit f403e0db17c184bb595ab0dd953ac809957ea768 [perl #82111] de-pessimise some my @array = ... Affected files ... Differences ... Inline Patchdiff --git a/pp_hot.c b/pp_hot.c
index 852ff80..f8a61cb 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -989,8 +989,19 @@ PP(pp_aassign)
/* If there's a common identifier on both sides we have to take
* special care that assigning the identifier on the left doesn't
* clobber a value on the right that's used later in the list.
+ * Don't bother if LHS is just an empty hash or array.
*/
- if (PL_op->op_private & (OPpASSIGN_COMMON)) {
+
+ if ( (PL_op->op_private & OPpASSIGN_COMMON)
+ && (
+ firstlelem != lastlelem
+ || ! ((sv = *firstlelem))
+ || SvMAGICAL(sv)
+ || ! (SvTYPE(sv) == SVt_PVAV || SvTYPE(sv) == SVt_PVHV)
+ || (SvTYPE(sv) == SVt_PVAV && AvFILL((AV*)sv) != -1)
+ || (SvTYPE(sv) == SVt_PVHV && HvKEYS((HV*)sv) != 0)
+ )
+ ) {
EXTEND_MORTAL(lastrelem - firstrelem + 1);
for (relem = firstrelem; relem <= lastrelem; relem++) {
if ((sv = *relem)) {
-- O Unicef Clearasil! |
|
@iabyn - Status changed from 'rejected' to 'resolved' |
From @demerphqOn 12 March 2011 23:12, Dave Mitchell <davem@iabyn.com> wrote:
Cool, thanks! Yves |
From @demerphqare you sure you pushed it? i dont see a mail, and when i pulled i On 12 March 2011 23:12, Dave Mitchell <davem@iabyn.com> wrote:
-- |
Migrated from rt.perl.org#82110 (status was 'resolved')
Searchable as RT82110$
The text was updated successfully, but these errors were encountered: