classification
Title: Superfluous call to isatty in open() when buffering >= 0
Type: performance Stage: resolved
Components: IO Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Dav1d, vstinner
Priority: normal Keywords: patch

Created on 2018-07-08 18:12 by Dav1d, last changed 2018-10-19 22:34 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8187 merged Dav1d, 2018-07-08 18:23
Messages (3)
msg321281 - (view) Author: David Herberth (Dav1d) * Date: 2018-07-08 18:12
_iomodule.c:_io_open_impl checks for isatty even if not necessary (when buffering >= 0).

Code: https://github.com/python/cpython/blob/c0ee341b29bd7d978b49272a2c0e2dcfa77404d5/Modules/_io/_iomodule.c#L392

    {
        PyObject *res = _PyObject_CallMethodId(raw, &PyId_isatty, NULL);
        if (res == NULL)
            goto error;
        isatty = PyLong_AsLong(res);
        Py_DECREF(res);
        if (isatty == -1 && PyErr_Occurred())
            goto error;
    }

    if (buffering == 1 || (buffering < 0 && isatty)) {
        buffering = -1;
        line_buffering = 1;
    }
    else
        line_buffering = 0;


Python Code to reproduce:

with open('foo', 'rb', buffering=0) as f:
    f.read()


This generates an error (can be seen with strace):

ioctl(5, TCGETS, 0x7ffef1435b60)  = -1 ENOTTY (Inappropriate ioctl for device)

I'll open a PR shortly.
msg328066 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-19 22:32
New changeset 8deab9672554edaf58f91e238cc899463d53f6ea by Victor Stinner (David Herberth) in branch 'master':
bpo-34070: open() only checks for isatty if buffering < 0 (GH-8187)
https://github.com/python/cpython/commit/8deab9672554edaf58f91e238cc899463d53f6ea
msg328067 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-10-19 22:34
David Herberth: Thanks for report the issue, I didn't noticed it previously even if I read strace output frequently! And thanks for the fix.


> This generates an error (can be seen with strace):
> ioctl(5, TCGETS, 0x7ffef1435b60)  = -1 ENOTTY (Inappropriate ioctl for device)

Honestly, I don't think that it's a major bug, so I suggest to not backport the change to 3.7 and older. I prefer to not change stable changes to avoid any risk of regression.
History
Date User Action Args
2018-10-19 22:34:22vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg328067

stage: patch review -> resolved
2018-10-19 22:32:08vstinnersetnosy: + vstinner
messages: + msg328066
2018-07-08 18:23:37Dav1dsetkeywords: + patch
stage: patch review
pull_requests: + pull_request7742
2018-07-08 18:12:53Dav1dcreate