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

[PATCH] Win32: stat() only after a failed open() on a module #14408

Closed
p5pRT opened this issue Jan 8, 2015 · 8 comments
Closed

[PATCH] Win32: stat() only after a failed open() on a module #14408

p5pRT opened this issue Jan 8, 2015 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 8, 2015

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

Searchable as RT123566$

@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2015

From @bulk88

Created by @bulk88

See attached patch. Not fully smoked. Slightly related to
https://rt.perl.org/Public/Bug/Display.html?id=121119

non existent modules and failed paths in @​INC should also be faster

before

11​:20​:03.0096859 PM perl.exe 3712 RegOpenKey
HKLM\Software\Wow6432Node\Perl NAME NOT FOUND
11​:20​:03.0110271 PM perl.exe 3712 CreateFile
C​:\p521\srcnew\lib\warnINVALIDings.pm NAME NOT FOUND Desired Access​:
Read Attributes, Synchronize, Disposition​: Open, Options​: Synchronous IO
Non-Alert, Non-Directory File, Attributes​: n/a, ShareMode​: None,
AllocationSize​: n/a
11​:20​:03.0122386 PM perl.exe 3712 CreateFile C​:\p521\srcnew\lib SUCCESS
Desired Access​: Read Data/List Directory, Synchronize, Disposition​:
Open, Options​: Directory, Synchronous IO Non-Alert, Attributes​: n/a,
ShareMode​: Read, Write, Delete, AllocationSize​: n/a, OpenResult​: Opened
11​:20​:03.0123769 PM perl.exe 3712 QueryDirectory
C​:\p521\srcnew\lib\warnINVALIDings.pm NO SUCH FILE Filter​:
warnINVALIDings.pm
11​:20​:03.0124862 PM perl.exe 3712 CloseFile C​:\p521\srcnew\lib SUCCESS
11​:20​:03.0137250 PM perl.exe 3712 QueryOpen
C​:\p521\srcnew\lib\warnINVALIDings.pm NAME NOT FOUND
11​:20​:03.0146135 PM perl.exe 3712 CreateFile
C​:\p521\srcnew\warnINVALIDings.pm NAME NOT FOUND Desired Access​: Read
Attributes, Synchronize, Disposition​: Open, Options​: Synchronous IO
Non-Alert, Non-Directory File, Attributes​: n/a, ShareMode​: None,
AllocationSize​: n/a
11​:20​:03.0155561 PM perl.exe 3712 CreateFile C​:\p521\srcnew SUCCESS
Desired Access​: Read Data/List Directory, Synchronize, Disposition​:
Open, Options​: Directory, Synchronous IO Non-Alert, Attributes​: n/a,
ShareMode​: Read, Write, Delete, AllocationSize​: n/a, OpenResult​: Opened
11​:20​:03.0156870 PM perl.exe 3712 QueryDirectory
C​:\p521\srcnew\warnINVALIDings.pm NO SUCH FILE Filter​: warnINVALIDings.pm
11​:20​:03.0158040 PM perl.exe 3712 CloseFile C​:\p521\srcnew SUCCESS
11​:20​:03.0168142 PM perl.exe 3712 QueryOpen
C​:\p521\srcnew\warnINVALIDings.pm NAME NOT FOUND
11​:20​:03.0177405 PM perl.exe 3712 RegOpenKey
HKLM\Software\Wow6432Node\Microsoft\Windows
NT\CurrentVersion\GRE_Initialize SUCCESS

after

11​:19​:25.8889882 PM perl.exe 3316 RegOpenKey
HKLM\Software\Wow6432Node\Perl NAME NOT FOUND
11​:19​:25.8904238 PM perl.exe 3316 CreateFile
C​:\p521\srcnewb4opt\lib\warnINVALIDings.pm NAME NOT FOUND Desired
Access​: Generic Read, Disposition​: Open, Options​: Synchronous IO
Non-Alert, Non-Directory File, Attributes​: N, ShareMode​: Read, Write,
AllocationSize​: n/a
11​:19​:25.8913463 PM perl.exe 3316 CreateFile
C​:\p521\srcnewb4opt\warnINVALIDings.pm NAME NOT FOUND Desired Access​:
Generic Read, Disposition​: Open, Options​: Synchronous IO Non-Alert,
Non-Directory File, Attributes​: N, ShareMode​: Read, Write,
AllocationSize​: n/a
11​:19​:25.8924115 PM perl.exe 3316 RegOpenKey
HKLM\Software\Wow6432Node\Microsoft\Windows
NT\CurrentVersion\GRE_Initialize SUCCESS

Perl Info

Flags:
                     category=core
                     severity=low

Site configuration information for perl 5.21.7:

Configured by Owner at Sat Nov 22 21:54:54 2014.

Summary of my perl5 (revision 5 version 21 subversion 7) configuration:
                   Local Commit: 1bce52df028aabe28c20b2d97949e35c17ea811e
                   Ancestor: 7072da8afeba4c87ae623cd913e274396ffcf1cd
                   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 -DNO_MATHOMS -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,
doublekind=3
                     d_longlong=undef, longlongsize=8, d_longdbl=define,
longdblsize=8,
longdblkind=0
                     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:
                     ce7a4d57d0acca9f39a84d36d708c4505dfe45ca
                     ca0b263f4b167ddf97416f657d79ab5bd3344357
                     08919bf863666074243240abbd19cd1a74cc7b74
                     b8a043377dbf39548709b107a11e5cc2714c0e9a
                     efa855eb5cffb7739616c295dd968d1510efeeb0
                     1d47d0b810e26d9a2f9101fb813bd5b3dd332cc9
                     3faca062ddb056db54f73fa55b0a9d473675dd33
                     0b3e905bda3e75ad948a1213f620656b60807393
                     1b1efc719fde05d215e5a13fb38c03e12a3aab08
                     1bce52df028aabe28c20b2d97949e35c17ea811e


@INC for perl 5.21.7:
                     ..\lib
                     C:/perl521/srcnewb4opt/lib
                     .


Environment for perl 5.21.7:
                     HOME (unset)
                     LANG (unset)
                     LANGUAGE (unset)
                     LD_LIBRARY_PATH (unset)
                     LOGDIR (unset)
                     PATH=C:\WINDOWS\system32;C:\Program Files\Microsoft
Visual
Studio
.NET 2003\Vc7\bin;C:\Program Files\Microsoft Visual Studio .NET
2003\Common7\IDE;C:\WINDOWS;C:\Program Files\Git\cmd;C:\Program
Files\Microsoft Visual Studio .NET 2003\Common7\Tools\bin;C:\perl\bin
                     PERL_BADLANG (unset)
                     PERL_JSON_BACKEND=Cpanel::JSON::XS
                     PERL_YAML_BACKEND=YAML
                     SHELL (unset)























@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2015

From @bulk88

0001-Win32-stat-only-after-a-failed-open-on-a-module.patch
From 25fb429defd25808bc2ab0d86cb48f187f9d380c Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Thu, 8 Jan 2015 00:10:13 -0500
Subject: [PATCH] Win32: stat() only after a failed open() on a module

See RT ticket for this patch for details.
---
 pod/perldelta.pod |  5 +++++
 pp_ctl.c          | 27 +++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index 7ee0ec4..5f0456d 100644
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -364,6 +364,11 @@ would uncondtionally have around a dozen warnings from hv_func.h.  These
 warnings have been silenced.  GCC all bitness and Visual C++ for Win32 were
 not affected.
 
+=item *
+
+Between 2 and 6 ms and 7 I/O calls have been saved per attempt to open a perl
+module for each path in C<@INC>.
+
 =back
 
 =back
diff --git a/pp_ctl.c b/pp_ctl.c
index f2c9856..d69710c 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3578,6 +3578,7 @@ S_check_type_and_open(pTHX_ SV *name)
 {
     Stat_t st;
     STRLEN len;
+    PerlIO * retio;
     const char *p = SvPV_const(name, len);
     int st_rc;
 
@@ -3592,6 +3593,11 @@ S_check_type_and_open(pTHX_ SV *name)
     if (!IS_SAFE_PATHNAME(p, len, "require"))
         return NULL;
 
+    /* on Win32 stat is expensive (it does an open() and close() twice and
+       a couple other IO calls), the open will fail with a dir on its own with
+       errno EACCES, so only do a stat to separate a dir from a real EACCES
+       caused by user perms */
+#ifndef WIN32
     /* we use the value of errno later to see how stat() or open() failed.
      * We don't want it set if the stat succeeded but we still failed,
      * such as if the name exists, but is a directory */
@@ -3602,12 +3608,29 @@ S_check_type_and_open(pTHX_ SV *name)
     if (st_rc < 0 || S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode)) {
 	return NULL;
     }
+#endif
 
 #if !defined(PERLIO_IS_STDIO)
-    return PerlIO_openn(aTHX_ ":", PERL_SCRIPT_MODE, -1, 0, 0, NULL, 1, &name);
+    retio = PerlIO_openn(aTHX_ ":", PERL_SCRIPT_MODE, -1, 0, 0, NULL, 1, &name);
 #else
-    return PerlIO_open(p, PERL_SCRIPT_MODE);
+    retio = PerlIO_open(p, PERL_SCRIPT_MODE);
+#endif
+#ifdef WIN32
+    /* EACCES stops the INC search early in pp_require to implement
+       feature RT #113422 */
+    if(!retio && errno == EACCES) { /* exists but probably a directory */
+	int eno;
+	st_rc = PerlLIO_stat(p, &st);
+	if (st_rc >= 0) {
+	    if(S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode))
+		eno = 0;
+	    else
+		eno = EACCES;
+	    errno = eno;
+	}
+    }
 #endif
+    return retio;
 }
 
 #ifndef PERL_DISABLE_PMC
-- 
1.8.0.msysgit.0

@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2015

From @bulk88

after.CSV

@p5pRT
Copy link
Author

p5pRT commented Jan 8, 2015

From @bulk88

before.CSV

@p5pRT
Copy link
Author

p5pRT commented Jan 11, 2015

From @bulk88

On Wed Jan 07 22​:10​:20 2015, bulk88 wrote​:

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

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

See attached patch. Not fully smoked. Slightly related to
https://rt.perl.org/Public/Bug/Display.html?id=121119

non existent modules and failed paths in @​INC should also be faster

Bump, any comments from Unix porters on whether the algorithm change would be beneficial on Unix, not just Win32 as the patch has it?

Any comments from Win32 porters who will commit this?

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Jan 12, 2015

From @tonycoz

On Sun Jan 11 02​:46​:02 2015, bulk88 wrote​:

See attached patch. Not fully smoked. Slightly related to
https://rt.perl.org/Public/Bug/Display.html?id=121119

non existent modules and failed paths in @​INC should also be faster

Bump, any comments from Unix porters on whether the algorithm change
would be beneficial on Unix, not just Win32 as the patch has it?

Opening a directory for read is successful on most POSIX systems.

Any comments from Win32 porters who will commit this?

Thanks, applied as d345f48.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jan 12, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Jan 12, 2015

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

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

1 participant