pythonGH-120754: Remove isatty call during regular read

For POSIX, TTYs are never regular files, so if the interpreter knows the
file is regular it doesn't need to do an additional system call to check
if the file is a tty.

The `open()` API requires a `stat` call at present in order to make sure
the file being opened isn't a directory. That result includes the file
mode which tells us if it is a regular file. There's a number of
attributes from the stat which are stashed one off currently, move to
stashing the whole object rather than just individual members.

The stat object is reasonably large and currently the
`stat_result.st_size` member cannot be modified from Python, which is
needed by the `_pyio` implementation, so make the whole stat object
optional. In the `_io` implementation this makes handling a stat
failure simpler. At present there is no explicit user call to clear it,
but if one is needed (ex. a program which has a lot of open FileIO
objects and the memory becomes a problem) it would be straightforward to
add. Ideally would be able to automatically clear (the values are
generally used during I/O object initialization and not after. After a
`write` they are no longer useful in current cases).

It is fairly common pattern to scan a directory, look at the `stat`
results (ex. is this file changed), and then open/read the file. In this
PR I didn't update open's API to allow passing in a stat result to use,
but that could be beneficial for some cases (ex. `importlib`).

With this change on my Linux machine reading a small plain text file is
down to 6 system calls.

```
openat(AT_FDCWD, "read_one.py", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=87, ...}) = 0
lseek(3, 0, SEEK_CUR)                   = 0
read(3, "from pathlib import Path\n\npath ="..., 88) = 87
read(3, "", 1)                          = 0
close(3)                                = 0
```

Author: cmaloney

Lib/_pyio.py

@@ -242,14 +242,7 @@ def open(file, mode="r", buffering=-1, encoding=None, errors=None,
             buffering = -1
             line_buffering = True
         if buffering < 0:
-            buffering = DEFAULT_BUFFER_SIZE
-            try:
-                bs = os.fstat(raw.fileno()).st_blksize
-            except (OSError, AttributeError):
-                pass
-            else:
-                if bs > 1:
-                    buffering = bs
+            buffering = raw._blksize
         if buffering < 0:
             raise ValueError("invalid buffering size")
         if buffering == 0:
@@ -1565,19 +1558,15 @@ def __init__(self, file, mode='r', closefd=True, opener=None):
                     os.set_inheritable(fd, False)
 
             self._closefd = closefd
-            fdfstat = os.fstat(fd)
+            self._stat_atopen = os.fstat(fd)
             try:
-                if stat.S_ISDIR(fdfstat.st_mode):
+                if stat.S_ISDIR(self._stat_atopen.st_mode):
                     raise IsADirectoryError(errno.EISDIR,
                                             os.strerror(errno.EISDIR), file)
             except AttributeError:
                 # Ignore the AttributeError if stat.S_ISDIR or errno.EISDIR
                 # don't exist.
                 pass
-            self._blksize = getattr(fdfstat, 'st_blksize', 0)
-            if self._blksize <= 1:
-                self._blksize = DEFAULT_BUFFER_SIZE
-            self._estimated_size = fdfstat.st_size
 
             if _setmode:
                 # don't translate newlines (\r\n <=> \n)
@@ -1623,6 +1612,13 @@ def __repr__(self):
             return ('<%s name=%r mode=%r closefd=%r>' %
                     (class_name, name, self.mode, self._closefd))
 
+    @property
+    def _blksize(self):
+        if self._stat_atopen and self._stat_atopen.st_blksize > 0:
+            return self._stat_atopen.st_blksize
+
+        return DEFAULT_BUFFER_SIZE
+
     def _checkReadable(self):
         if not self._readable:
             raise UnsupportedOperation('File not open for reading')
@@ -1655,16 +1651,16 @@ def readall(self):
         """
         self._checkClosed()
         self._checkReadable()
-        if self._estimated_size <= 0:
+        if not self._stat_atopen or self._stat_atopen.st_size <= 0:
             bufsize = DEFAULT_BUFFER_SIZE
         else:
-            bufsize = self._estimated_size + 1
+            bufsize = self._stat_atopen.st_size + 1
 
-            if self._estimated_size > 65536:
+            if self._stat_atopen.st_size > 65536:
                 try:
                     pos = os.lseek(self._fd, 0, SEEK_CUR)
-                    if self._estimated_size >= pos:
-                        bufsize = self._estimated_size - pos + 1
+                    if self._stat_atopen.st_size >= pos:
+                        bufsize = self._stat_atopen.st_size - pos + 1
                 except OSError:
                     pass
 
@@ -1742,7 +1738,7 @@ def truncate(self, size=None):
         if size is None:
             size = self.tell()
         os.ftruncate(self._fd, size)
-        self._estimated_size = size
+        self._stat_atopen = None
         return size
 
     def close(self):
@@ -1788,6 +1784,10 @@ def fileno(self):
     def isatty(self):
         """True if the file is connected to a TTY device."""
         self._checkClosed()
+        # Regular files are never TTYs, save a syscall for common "read file off
+        # disk" case.
+        if self._stat_atopen and stat.S_ISREG(self._stat_atopen.st_mode):
+            return False
         return os.isatty(self._fd)
 
     @property

Modules/_io/fileio.c

@@ -74,8 +74,7 @@ typedef struct {
     signed int seekable : 2; /* -1 means unknown */
     unsigned int closefd : 1;
     char finalizing;
-    unsigned int blksize;
-    Py_off_t estimated_size;
+    struct _Py_stat_struct *stat_atopen;
     PyObject *weakreflist;
     PyObject *dict;
 } fileio;
@@ -199,8 +198,7 @@ fileio_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         self->writable = 0;
         self->appending = 0;
         self->seekable = -1;
-        self->blksize = 0;
-        self->estimated_size = -1;
+        self->stat_atopen = NULL;
         self->closefd = 1;
         self->weakreflist = NULL;
     }
@@ -256,7 +254,6 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
 #elif !defined(MS_WINDOWS)
     int *atomic_flag_works = NULL;
 #endif
-    struct _Py_stat_struct fdfstat;
     int fstat_result;
     int async_err = 0;
 
@@ -454,9 +451,13 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
 #endif
     }
 
-    self->blksize = DEFAULT_BUFFER_SIZE;
+    self->stat_atopen = PyMem_New(struct _Py_stat_struct, 1);
+    if (self->stat_atopen == NULL) {
+        PyErr_Format(PyExc_MemoryError, "Unable to allocate space for stat result");
+        goto error;
+    }
     Py_BEGIN_ALLOW_THREADS
-    fstat_result = _Py_fstat_noraise(self->fd, &fdfstat);
+    fstat_result = _Py_fstat_noraise(self->fd, self->stat_atopen);
     Py_END_ALLOW_THREADS
     if (fstat_result < 0) {
         /* Tolerate fstat() errors other than EBADF.  See Issue #25717, where
@@ -471,25 +472,21 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
 #endif
             goto error;
         }
+
+        PyMem_Free(self->stat_atopen);
+        self->stat_atopen = NULL;
     }
     else {
 #if defined(S_ISDIR) && defined(EISDIR)
         /* On Unix, open will succeed for directories.
            In Python, there should be no file objects referring to
            directories, so we need a check.  */
-        if (S_ISDIR(fdfstat.st_mode)) {
+        if (S_ISDIR(self->stat_atopen->st_mode)) {
             errno = EISDIR;
             PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
             goto error;
         }
 #endif /* defined(S_ISDIR) */
-#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
-        if (fdfstat.st_blksize > 1)
-            self->blksize = fdfstat.st_blksize;
-#endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */
-        if (fdfstat.st_size < PY_SSIZE_T_MAX) {
-            self->estimated_size = (Py_off_t)fdfstat.st_size;
-        }
     }
 
 #if defined(MS_WINDOWS) || defined(__CYGWIN__)
@@ -521,6 +518,10 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
         internal_close(self);
         _PyErr_ChainExceptions1(exc);
     }
+    if (self->stat_atopen != NULL) {
+        PyMem_Free(self->stat_atopen);
+        self->stat_atopen = NULL;
+    }
 
  done:
 #ifdef MS_WINDOWS
@@ -553,6 +554,10 @@ fileio_dealloc(fileio *self)
     if (_PyIOBase_finalize((PyObject *) self) < 0)
         return;
     _PyObject_GC_UNTRACK(self);
+    if (self->stat_atopen != NULL) {
+        PyMem_Free(self->stat_atopen);
+        self->stat_atopen = NULL;
+    }
     if (self->weakreflist != NULL)
         PyObject_ClearWeakRefs((PyObject *) self);
     (void)fileio_clear(self);
@@ -725,7 +730,12 @@ _io_FileIO_readall_impl(fileio *self)
         return err_closed();
     }
 
-    end = self->estimated_size;
+    if (self->stat_atopen) {
+        end = (Py_off_t)Py_MIN(self->stat_atopen->st_size, PY_SSIZE_T_MAX);
+    } else {
+        end = -1;
+    }
+
     if (end <= 0) {
         /* Use a default size and resize as needed. */
         bufsize = SMALLCHUNK;
@@ -1098,7 +1108,10 @@ _io_FileIO_truncate_impl(fileio *self, PyTypeObject *cls, PyObject *posobj)
     estimate, that it is much larger than the actual size can result in a
     significant over allocation and sometimes a MemoryError / running out of
     memory. */
-    self->estimated_size = pos;
+    if (self->stat_atopen) {
+        PyMem_Free(self->stat_atopen);
+        self->stat_atopen = NULL;
+    }
 
     return posobj;
 }
@@ -1179,6 +1192,11 @@ _io_FileIO_isatty_impl(fileio *self)
 
     if (self->fd < 0)
         return err_closed();
+
+    if (self->stat_atopen && S_ISREG(self->stat_atopen->st_mode)) {
+        return Py_False;
+    }
+
     Py_BEGIN_ALLOW_THREADS
     _Py_BEGIN_SUPPRESS_IPH
     res = isatty(self->fd);
@@ -1229,16 +1247,28 @@ get_mode(fileio *self, void *closure)
     return PyUnicode_FromString(mode_string(self));
 }
 
+static PyObject *
+get_blksize(fileio *self, void *closure)
+{
+#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
+    if (self->stat_atopen && self->stat_atopen->st_blksize > 1) {
+        return PyLong_FromLong(self->stat_atopen->st_blksize);
+    }
+#endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */
+    return PyLong_FromLong(DEFAULT_BUFFER_SIZE);
+}
+
+
 static PyGetSetDef fileio_getsetlist[] = {
     {"closed", (getter)get_closed, NULL, "True if the file is closed"},
     {"closefd", (getter)get_closefd, NULL,
         "True if the file descriptor will be closed by close()."},
     {"mode", (getter)get_mode, NULL, "String giving the file mode"},
+    {"_blksize", (getter)get_blksize, NULL, "blksize to use for io"},
     {NULL},
 };
 
 static PyMemberDef fileio_members[] = {
-    {"_blksize", Py_T_UINT, offsetof(fileio, blksize), 0},
     {"_finalizing", Py_T_BOOL, offsetof(fileio, finalizing), 0},
     {"__weaklistoffset__", Py_T_PYSSIZET, offsetof(fileio, weakreflist), Py_READONLY},
     {"__dictoffset__", Py_T_PYSSIZET, offsetof(fileio, dict), Py_READONLY},