Skip to content
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

Jump to bottom

[PATCH] refactor av_delete #13659

Closed
p5pRT opened this issue Mar 13, 2014 · 7 comments
Closed

[PATCH] refactor av_delete #13659

p5pRT opened this issue Mar 13, 2014 · 7 comments
Labels
distro-mswin32 type-core

Comments

@p5pRT
Copy link

p5pRT commented Mar 13, 2014

Migrated from rt.perl.org#121436 (status was 'resolved')

Searchable as RT121436$
@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2014

From @bulk88

Created by @bulk88

See attached patch.

Perl Info 

Flags:
             category=core
             severity=low

Site configuration information for perl 5.19.7:

Configured by Owner at Thu Nov 28 02:32:44 2013.

Summary of my perl5 (revision 5 version 19 subversion 7) configuration:
           Derived from: 8f47723e28b75530b743941cdd8b07f849ec48e2
           Ancestor: 1061065f7a09399eefb50e9a035502621722bcc0
           Platform:
             osname=MSWin32, osvers=5.1, archname=MSWin32-x86-multi-thread
             uname=''
             config_args='undef'
             hint=recommended, useposix=true, d_sigaction=undef
             useithreads=define, usemultiplicity=define
             useperlio=define, d_sfio=undef, uselargefiles=define,
usesocks=undef
             use64bitint=undef, use64bitall=undef, uselongdouble=undef
             usemymalloc=n, bincompat5005=undef
           Compiler:
             cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -G7 -GL
-DWIN32 -D_CONSOLE -DNO_STRICT  -DPERL_TEXTMODE_SCRIPTS
-DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT
-DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T',
             optimize='-O1 -MD -Zi -DNDEBUG -G7 -GL',
             cppflags='-DWIN32'
             ccversion='13.10.6030', gccversion='', gccosandvers=''
             intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
             d_longlong=undef, longlongsize=8, d_longdbl=define,
longdblsize=8
             ivtype='long', ivsize=4, nvtype='double', nvsize=8,
Off_t='__int64',
lseeksize=8
             alignbytes=8, prototype=define
           Linker and Libraries:
             ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf
-ltcg  -libpath:"c:\perl519\lib\CORE"  -machine:x86'
             libpth="C:\Program Files\Microsoft Visual Studio .NET
2003\VC7\lib"
             libs=oldnames.lib kernel32.lib user32.lib gdi32.lib 
winspool.lib
comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib  version.lib
odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
             perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib
winspool.lib  comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib  netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib
version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib
             libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl519.lib
             gnulibc_version=''
           Dynamic Linking:
             dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
             cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug
-opt:ref,icf -ltcg  -libpath:"c:\perl519\lib\CORE"  -machine:x86'

Locally applied patches:
             uncommitted-changes
             8f47723e28b75530b743941cdd8b07f849ec48e2


@INC for perl 5.19.7:
             C:/perl519/site/lib
             C:/perl519/lib
             .


Environment for perl 5.19.7:
             HOME (unset)
             LANG (unset)
             LANGUAGE (unset)
             LD_LIBRARY_PATH (unset)
             LOGDIR (unset)
             PATH=C:\perl519\bin;C:\Program Files\Microsoft Visual Studio
.NET
2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET
2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET
2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET
2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem;
             PERL_BADLANG (unset)
             SHELL (unset)

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2014

From @bulk88

0001-refactor-av_delete.patch 
From 5d065ae2a1ec0881fd2ece2f97b5511dd4adff11 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Thu, 13 Mar 2014 02:51:51 -0400
Subject: [PATCH] refactor av_delete

Some bad code generation/bad optimization from VC caused me to clean this
up. Dont write AvARRAY(av)[key] = NULL twice. It appears in 2 branches
just as written VC asm wise. Don't call sv_2mortal on a NULL SV*. It
works but less efficient. On VC2008 x64 -O1 this func dropped from 0x208
to 0x202 bytes of machine code after this patch.
---
 av.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/av.c b/av.c
index 5ef3a55..c08d2c2 100644
--- a/av.c
+++ b/av.c
@@ -889,23 +889,23 @@ Perl_av_delete(pTHX_ AV *av, SSize_t key, I32 flags)
 	if (!AvREAL(av) && AvREIFY(av))
 	    av_reify(av);
 	sv = AvARRAY(av)[key];
+	AvARRAY(av)[key] = NULL;
 	if (key == AvFILLp(av)) {
-	    AvARRAY(av)[key] = NULL;
 	    do {
 		AvFILLp(av)--;
 	    } while (--key >= 0 && !AvARRAY(av)[key]);
 	}
-	else
-	    AvARRAY(av)[key] = NULL;
 	if (SvSMAGICAL(av))
 	    mg_set(MUTABLE_SV(av));
     }
-    if (flags & G_DISCARD) {
-	SvREFCNT_dec(sv);
-	sv = NULL;
+    if(sv != NULL) {
+	if (flags & G_DISCARD) {
+	    SvREFCNT_dec_NN(sv);
+	    return NULL;
+	}
+	else if (AvREAL(av))
+	    sv_2mortal(sv);
     }
-    else if (AvREAL(av))
-	sv = sv_2mortal(sv);
     return sv;
 }
 
-- 
1.8.0.msysgit.0

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2014

From @bulk88

On Thu Mar 13 11​:06​:24 2014, bulk88 wrote​:

This is a bug report for perl from bulk88@​hotmail.com,
generated with the help of perlbug 1.39 running under perl 5.19.7.

-----------------------------------------------------------------
[Please describe your issue here]

See attached patch.

I didn't smoke this due to another bug I'm dealing with, so you may find out it fails when you test it.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Apr 2, 2014

From @tonycoz

On Thu Mar 13 11​:06​:24 2014, bulk88 wrote​:

See attached patch.

I'll apply this after 5.20, added as a 5.21.1 blocker.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 2, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

From @tonycoz

On Thu Mar 13 11​:06​:24 2014, bulk88 wrote​:

See attached patch.

Thanks, applied as 725995b.

Tony

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

@tonycoz - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed May 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
distro-mswin32 type-core
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@p5pRT