develooper Front page | perl.perl5.porters | Postings from January 2013

Re: [perl #116188] [PATCH] hv.c: add some SvREFCNT_dec_NN andS_hv_free_ent_ret NULL check removal

Andy Dougherty
January 25, 2013 20:43
Re: [perl #116188] [PATCH] hv.c: add some SvREFCNT_dec_NN andS_hv_free_ent_ret NULL check removal
Message ID:
On Sun, 23 Dec 2012, bulk88 wrote:

> # New Ticket Created by  bulk88 
> # Please include the string:  [perl #116188]
> # in the subject line of all future correspondence about this issue. 
> # <URL: >

[minor nit:  This patch breaks a debugging build because embed.fnc 
incorrectly spells the second argument as 'entryK', while the actual 
functions use 'entry'.  I fixed this in commit 
c77ce6f12343471289c57516378e4af4ccce4b6c.  I then tweaked this patch
to use 'entry' instead of 'entryK'.]

I tried this patch and tested it on Linux/amd64 with gcc-4.7.2,
with various optimizer settings.

It looks like this patch is doing good things -- avoiding redundant NULL 
checks, avoiding duplicating a test, removing an unnecessary context, 
etc., and doing it in a set of functions that gets called fairly often.  
However, I found that it made no difference in performance in running a 
pattern matching program similar to metaconfig, and (under -O3) the new 
object files are actually larger than the old ones.

In both cases, gcc inlines the S_hv_free_ent_ret function anyway, and the 
resulting Perl_hv_free_ent function is exactly the same size before and 
after the patch.

I also tested with Sun's cc compiler.  It ended up giving the same 
performance and the same object size before and after the patch.

So I'm not sure if it's worth applying or not.  What do you think?

    Andy Dougherty Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About