Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.9k stars 540 forks source link

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

Closed p5pRT closed 9 years ago

p5pRT commented 9 years ago

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

Searchable as RT123566$

p5pRT commented 9 years ago

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 commented 9 years ago

From @bulk88

0001-Win32-stat-only-after-a-failed-open-on-a-module.patch ```diff From 25fb429defd25808bc2ab0d86cb48f187f9d380c Mon Sep 17 00:00:00 2001 From: Daniel Dragan 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 commented 9 years ago

From @bulk88

after.CSV

p5pRT commented 9 years ago

From @bulk88

before.CSV

p5pRT commented 9 years ago

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 commented 9 years ago

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 d345f4877521ed3ad5ad337f7e87a95625df05c0.

Tony

p5pRT commented 9 years ago

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

p5pRT commented 9 years ago

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