gh-101196: Make isdir/isfile/exists faster on Windows (GH-101324)

mdboom · eryksun · web-flow · commit 86ebd5c3fa9a · 2023-02-08T14:34:24.000Z
Co-authored-by: Eryk Sun &lt;eryksun@gmail.com&gt;
diff --git a/Include/pyport.h b/Include/pyport.h
@@ -247,6 +247,10 @@ typedef Py_ssize_t Py_ssize_clean_t;
 #define S_ISCHR(x) (((x) & S_IFMT) == S_IFCHR)
 #endif
 
+#ifndef S_ISLNK
+#define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
+#endif
+
 #ifdef __cplusplus
 /* Move this down here since some C++ #include's don't like to be included
    inside an extern "C" */
diff --git a/Lib/genericpath.py b/Lib/genericpath.py
@@ -7,7 +7,7 @@
 import stat
 
 __all__ = ['commonprefix', 'exists', 'getatime', 'getctime', 'getmtime',
-           'getsize', 'isdir', 'isfile', 'samefile', 'sameopenfile',
+           'getsize', 'isdir', 'isfile', 'islink', 'samefile', 'sameopenfile',
            'samestat']
 
 
@@ -45,6 +45,18 @@ def isdir(s):
     return stat.S_ISDIR(st.st_mode)
 
 
+# Is a path a symbolic link?
+# This will always return false on systems where os.lstat doesn't exist.
+
+def islink(path):
+    """Test whether a path is a symbolic link"""
+    try:
+        st = os.lstat(path)
+    except (OSError, ValueError, AttributeError):
+        return False
+    return stat.S_ISLNK(st.st_mode)
+
+
 def getsize(filename):
     """Return the size of a file, reported by os.stat()."""
     return os.stat(filename).st_size
diff --git a/Lib/ntpath.py b/Lib/ntpath.py
@@ -276,19 +276,6 @@ def dirname(p):
     """Returns the directory component of a pathname"""
     return split(p)[0]
 
-# Is a path a symbolic link?
-# This will always return false on systems where os.lstat doesn't exist.
-
-def islink(path):
-    """Test whether a path is a symbolic link.
-    This will always return false for Windows prior to 6.0.
-    """
-    try:
-        st = os.lstat(path)
-    except (OSError, ValueError, AttributeError):
-        return False
-    return stat.S_ISLNK(st.st_mode)
-
 
 # Is a path a junction?
 
@@ -870,11 +857,13 @@ def commonpath(paths):
 
 
 try:
-    # The genericpath.isdir implementation uses os.stat and checks the mode
-    # attribute to tell whether or not the path is a directory.
-    # This is overkill on Windows - just pass the path to GetFileAttributes
-    # and check the attribute from there.
-    from nt import _isdir as isdir
+    # The isdir(), isfile(), islink() and exists() implementations in
+    # genericpath use os.stat(). This is overkill on Windows. Use simpler
+    # builtin functions if they are available.
+    from nt import _path_isdir as isdir
+    from nt import _path_isfile as isfile
+    from nt import _path_islink as islink
+    from nt import _path_exists as exists
 except ImportError:
-    # Use genericpath.isdir as imported above.
+    # Use genericpath.* as imported above
     pass
diff --git a/Lib/posixpath.py b/Lib/posixpath.py
@@ -187,18 +187,6 @@ def dirname(p):
     return head
 
 
-# Is a path a symbolic link?
-# This will always return false on systems where os.lstat doesn't exist.
-
-def islink(path):
-    """Test whether a path is a symbolic link"""
-    try:
-        st = os.lstat(path)
-    except (OSError, ValueError, AttributeError):
-        return False
-    return stat.S_ISLNK(st.st_mode)
-
-
 # Is a path a junction?
 
 def isjunction(path):
diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py
@@ -1,9 +1,10 @@
+import inspect
 import ntpath
 import os
 import sys
 import unittest
 import warnings
-from test.support import os_helper
+from test.support import cpython_only, os_helper
 from test.support import TestFailed, is_emscripten
 from test.support.os_helper import FakePath
 from test import test_genericpath
@@ -938,6 +939,35 @@ def test_isjunction(self):
                 self.assertFalse(ntpath.isjunction('tmpdir'))
                 self.assertPathEqual(ntpath.realpath('testjunc'), ntpath.realpath('tmpdir'))
 
+    @unittest.skipIf(sys.platform != 'win32', "drive letters are a windows concept")
+    def test_isfile_driveletter(self):
+        drive = os.environ.get('SystemDrive')
+        if drive is None or len(drive) != 2 or drive[1] != ':':
+            raise unittest.SkipTest('SystemDrive is not defined or malformed')
+        self.assertFalse(os.path.isfile('\\\\.\\' + drive))
+
+    @unittest.skipIf(sys.platform != 'win32', "windows only")
+    def test_con_device(self):
+        self.assertFalse(os.path.isfile(r"\\.\CON"))
+        self.assertFalse(os.path.isdir(r"\\.\CON"))
+        self.assertFalse(os.path.islink(r"\\.\CON"))
+        self.assertTrue(os.path.exists(r"\\.\CON"))
+
+    @unittest.skipIf(sys.platform != 'win32', "Fast paths are only for win32")
+    @cpython_only
+    def test_fast_paths_in_use(self):
+        # There are fast paths of these functions implemented in posixmodule.c.
+        # Confirm that they are being used, and not the Python fallbacks in
+        # genericpath.py.
+        self.assertTrue(os.path.isdir is nt._path_isdir)
+        self.assertFalse(inspect.isfunction(os.path.isdir))
+        self.assertTrue(os.path.isfile is nt._path_isfile)
+        self.assertFalse(inspect.isfunction(os.path.isfile))
+        self.assertTrue(os.path.islink is nt._path_islink)
+        self.assertFalse(inspect.isfunction(os.path.islink))
+        self.assertTrue(os.path.exists is nt._path_exists)
+        self.assertFalse(inspect.isfunction(os.path.exists))
+
 
 class NtCommonTest(test_genericpath.CommonTest, unittest.TestCase):
     pathmodule = ntpath
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -742,6 +742,7 @@ def test_access_denied(self):
         )
         result = os.stat(fname)
         self.assertNotEqual(result.st_size, 0)
+        self.assertTrue(os.path.isfile(fname))
 
     @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
     def test_stat_block_device(self):
@@ -2860,6 +2861,7 @@ def test_appexeclink(self):
             self.assertEqual(st, os.stat(alias))
             self.assertFalse(stat.S_ISLNK(st.st_mode))
             self.assertEqual(st.st_reparse_tag, stat.IO_REPARSE_TAG_APPEXECLINK)
+            self.assertTrue(os.path.isfile(alias))
             # testing the first one we see is sufficient
             break
         else:
diff --git a/Misc/NEWS.d/next/Windows/2023-01-25-11-33-54.gh-issue-101196.wAX_2g.rst b/Misc/NEWS.d/next/Windows/2023-01-25-11-33-54.gh-issue-101196.wAX_2g.rst
@@ -0,0 +1,3 @@
+The functions ``os.path.isdir``, ``os.path.isfile``, ``os.path.islink`` and
+``os.path.exists`` are now 13% to 28% faster on Windows, by making fewer Win32
+API calls.
diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+The functions ``os.path.isdir``, ``os.path.isfile``, ``os.path.islink`` and
	`2`	+``os.path.exists`` are now 13% to 28% faster on Windows, by making fewer Win32
	`3`	`+API calls.`