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

commit "replace many SvTYPE assertions with lookup tables" adds unused on non-DEBUGGING tables #13976

Closed
p5pRT opened this issue Jul 10, 2014 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 10, 2014

Migrated from rt.perl.org#122262 (status was 'open')

Searchable as RT122262$

@p5pRT
Copy link
Author

p5pRT commented Jul 10, 2014

From @bulk88

Created by @bulk88

f1fb874 added a bunch of tables that are exported/extern in 5.15.0 . These tables are only used on DEBUGGING builds. The declarations need to be ifdefed away on non-DEBUGGING and a skip added to makedef.pl. Removing the tables will save 0x10*6 bytes of C tables/arrays from the image+exporting/symbol name string overhead.

Perl Info

Flags:
          category=core
          severity=low

Site configuration information for perl 5.21.1:

Configured by Owner at Wed May 28 03:57:28 2014.

Summary of my perl5 (revision 5 version 21 subversion 1) configuration:
        Local Commit: 1abbcfa06576bf8a6937c566bb4d18ba803b59d8
        Ancestor: 234105dd8a732de5fb48ccb1838c99281f89f669
        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
          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:\perl521\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=perl521.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:\perl521\lib\CORE"  -machine:x86'

Locally applied patches:
          7494266ea378a3cdc4bfd51725012c1e432db0f1
          61961437d9453dd0d4053ad100e97a029a24edbb
          cd30b936fc5177ce169d776445d09c9898c15da1
          1abbcfa06576bf8a6937c566bb4d18ba803b59d8


@INC for perl 5.21.1:
          C:/perl521/site/lib
          C:/perl521/lib
          .


Environment for perl 5.21.1:
          HOME (unset)
          LANG (unset)
          LANGUAGE (unset)
          LD_LIBRARY_PATH (unset)
          LOGDIR (unset)
          PATH=C:\perl521\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 Jul 10, 2014

From @bulk88

0001-static-misc-things-in-dist-threads-threads.xs.patch
>From c16920bcbecb9e90f35c8bcac8dd88a7058c39c5 Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Sun, 6 Jul 2014 02:59:02 -0400
Subject: [PATCH] static misc things in /dist/threads/threads.xs

there is no reason for these functions to be exported on a linux build,
since they aren't exported on Win32, also consting ithread_vtbl allows
more memory to be shared between perl procs
---
 dist/threads/lib/threads.pm |    2 +-
 dist/threads/threads.xs     |   10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/dist/threads/lib/threads.pm b/dist/threads/lib/threads.pm
index c395d7b..ff41a20 100644
--- a/dist/threads/lib/threads.pm
+++ b/dist/threads/lib/threads.pm
@@ -5,7 +5,7 @@ use 5.008;
 use strict;
 use warnings;
 
-our $VERSION = '1.94';
+our $VERSION = '1.95';
 my $XS_VERSION = $VERSION;
 $VERSION = eval $VERSION;
 
diff --git a/dist/threads/threads.xs b/dist/threads/threads.xs
index 6175ba7..5b44a38 100644
--- a/dist/threads/threads.xs
+++ b/dist/threads/threads.xs
@@ -346,7 +346,7 @@ S_exit_warning(pTHX)
 /* Called from perl_destruct() in each thread.  If it's the main thread,
  * stop it from freeing everything if there are other threads still running.
  */
-int
+STATIC int
 Perl_ithread_hook(pTHX)
 {
     dMY_POOL;
@@ -356,7 +356,7 @@ Perl_ithread_hook(pTHX)
 
 /* MAGIC (in mg.h sense) hooks */
 
-int
+STATIC int
 ithread_mg_get(pTHX_ SV *sv, MAGIC *mg)
 {
     ithread *thread = (ithread *)mg->mg_ptr;
@@ -365,7 +365,7 @@ ithread_mg_get(pTHX_ SV *sv, MAGIC *mg)
     return (0);
 }
 
-int
+STATIC int
 ithread_mg_free(pTHX_ SV *sv, MAGIC *mg)
 {
     ithread *thread = (ithread *)mg->mg_ptr;
@@ -375,7 +375,7 @@ ithread_mg_free(pTHX_ SV *sv, MAGIC *mg)
     return (0);
 }
 
-int
+STATIC int
 ithread_mg_dup(pTHX_ MAGIC *mg, CLONE_PARAMS *param)
 {
     PERL_UNUSED_ARG(param);
@@ -383,7 +383,7 @@ ithread_mg_dup(pTHX_ MAGIC *mg, CLONE_PARAMS *param)
     return (0);
 }
 
-MGVTBL ithread_vtbl = {
+STATIC const MGVTBL ithread_vtbl = {
     ithread_mg_get,     /* get */
     0,                  /* set */
     0,                  /* len */
-- 
1.7.9.msysgit.0


@p5pRT
Copy link
Author

p5pRT commented Jul 10, 2014

From @tonycoz

On Wed Jul 09 21​:35​:32 2014, bulk88 wrote​:

f1fb874
added a bunch of tables that are exported/extern in 5.15.0 . These
tables are only used on DEBUGGING builds. The declarations need to be
ifdefed away on non-DEBUGGING and a skip added to makedef.pl. Removing
the tables will save 0x10*6 bytes of C tables/arrays from the
image+exporting/symbol name string overhead.

You attached the following patch​:

From c16920bcbecb9e90f35c8bcac8dd88a7058c39c5 Mon Sep 17 00​:00​:00 2001
From​: Daniel Dragan <bulk88@​hotmail.com>
Date​: Sun, 6 Jul 2014 02​:59​:02 -0400
Subject​: [PATCH] static misc things in /dist/threads/threads.xs

there is no reason for these functions to be exported on a linux build,
since they aren't exported on Win32, also consting ithread_vtbl allows
more memory to be shared between perl procs


which was applied as e45636e.

Was there meant to be another patch?

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 10, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Jul 10, 2014

From @bulk88

On Wed Jul 09 22​:12​:11 2014, tonyc wrote​:

Was there meant to be another patch?

Tony

Whoops. There is no patch. I dont have the time ATM to pursue this, so I'm making it a plain old ticket/report.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2014

From @tonycoz

On Wed Jul 09 21​:35​:32 2014, bulk88 wrote​:

http​://perl5.git.perl.org/perl.git/commitdiff/f1fb87419225265377ea1d91eac0dc99547137a6
added a bunch of tables that are exported/extern in 5.15.0 . These
tables are only used on DEBUGGING builds. The declarations need to be
ifdefed away on non-DEBUGGING and a skip added to makedef.pl. Removing
the tables will save 0x10*6 bytes of C tables/arrays from the
image+exporting/symbol name string overhead.

Unfortunately parts of re (ext/re/) are built with DEBUGGING defined, so these arrays are uses there.
Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2014

From @tonycoz

On Tue Oct 21 16​:41​:36 2014, tonyc wrote​:

so these arrays are uses there.

Maybe, one day, I'll read what I type.

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 14, 2014

From @bulk88

On Tue Oct 21 16​:41​:36 2014, tonyc wrote​:

On Wed Jul 09 21​:35​:32 2014, bulk88 wrote​:

http​://perl5.git.perl.org/perl.git/commitdiff/f1fb87419225265377ea1d91eac0dc99547137a6
added a bunch of tables that are exported/extern in 5.15.0 . These
tables are only used on DEBUGGING builds. The declarations need to be
ifdefed away on non-DEBUGGING and a skip added to makedef.pl.
Removing
the tables will save 0x10*6 bytes of C tables/arrays from the
image+exporting/symbol name string overhead.

Unfortunately parts of re (ext/re/) are built with DEBUGGING defined,
so these arrays are uses there.
Tony

I tried to move the PL_valid* tables to be per module static on non-DEBUGGING core but DEBUGGING module modules (cough cough re​::*) but I discovered if core is non-DEBUGGING, re.dll doesn't import PL_valid_* and assert(). Is that a bug? Is it a Win32 specific bug? should re​:: on non-DEBUGGING core have assert() on? It probably has something to do with the -DNDEBUG that Win32 builds do but Unix dont.

--
bulk88 ~ bulk88 at hotmail.com

@toddr
Copy link
Member

toddr commented Feb 13, 2020

Given the submitted patch in this case is applied, I am closing this case. @bulk88 please submit a pull request if you have further changes. Thanks!

@toddr toddr closed this as completed Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants