[issue21679] Prevent extraneous fstat during open()

[Martin Panter]

Martin Panter added the comment:

The change made here no longer tolerates fstat() returning an error, which 
seems to be the cause of Issue 25717.

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Roundup Robot]

Roundup Robot added the comment:

New changeset 3b5279b5bfd1 by Antoine Pitrou in branch 'default':
Issue #21679: Prevent extraneous fstat() calls during open().  Patch by 
Bohuslav Kabrda.
http://hg.python.org/cpython/rev/3b5279b5bfd1

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Antoine Pitrou]

Antoine Pitrou added the comment:

Thank you very much. I've committed the patch to the default branch (I've just 
moved the _blksize test to a separate method).

--
resolution:  - fixed
stage:  - resolved
status: open - closed
versions:  -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

Bohuslav Slavek Kabrda added the comment:

I'm attaching fourth version of the patch. Changes:
- fileio's _blksize member is now of type T_UINT
- extended test_fileio.AutoFileTests.testAttributes to also test _blksize value

--
Added file: 
http://bugs.python.org/file35702/python3-remove-extraneous-fstat-on-file-open-v4.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

Bohuslav Slavek Kabrda added the comment:

Thanks, Antoine. So, is there anything else that should be done about the patch 
so that it gets accepted?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

Bohuslav Slavek Kabrda added the comment:

So, as pointed out by haypo, blksize_t is actually signed long on Linux. This 
means that my patch (as well as the current code) is not right.
Both with and without my patch, io_open function uses int to store blksize_t 
and it also passes it to one of PyBuffered{Random,Reader,Writer}_Type. These 
three however use Py_ssize_t, which is inconsistent with io_open, and it's not 
correct, too.
I'm unsure how to proceed here - should I fix buffer size types throughout the 
_io module to long and submit one big patch? It doesn't feel right to put two 
not-very-related changes into one patch (I'm afraid that changing buffer sizes 
to long everywhere will result in a rather large patch). Or should I first 
submit a patch that fixes the long usage and then rebase the patch attached to 
this issue on top of it?

Thanks a lot.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Antoine Pitrou]

Antoine Pitrou added the comment:

I think it's ok to keep the block size an int for now. It would be surprising 
for a block size to be more than 2 GB, IMO.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Josh Rosenberg]

Changes by Josh Rosenberg shadowranger+pyt...@gmail.com:


--
nosy: +josh.rosenberg

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

Bohuslav Slavek Kabrda added the comment:

Again, thanks for the review. It's true that HAVE_FSTAT can be defined without 
stat structure containing st_blksize. I added an ifdef 
HAVE_STRUCT_STAT_ST_BLKSIZE for that. Attaching third version of the patch, 
hopefully everything will be ok now.

--
Added file: 
http://bugs.python.org/file35548/python3-remove-extraneous-fstat-on-file-open-v3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

Bohuslav Slavek Kabrda added the comment:

Thanks a lot for the code review! I'm attaching a revised version of the patch. 
Fixes I made:

- added check whether PyLong_AsLong returned an error
- removed ADD_INTERNED(_blksize) and PyObject *_PyIO_str__blksize; - I 
noticed that these are only necessary when exported by _iomodule.h, which isn't 
needed for _blksize ATM
- moved blksize to a place of fileio structure where it won't create 
unnecessary padding

I hope attaching the version 2 of the patch here is ok, if I should have 
attached it in the code review tool somehow, please let me know.

--
Added file: 
http://bugs.python.org/file35540/python3-remove-extraneous-fstat-on-file-open-v2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Bohuslav Slavek Kabrda]

New submission from Bohuslav Slavek Kabrda:

Hi,
with Python 3.3/3.4, I noticed that there are lots of syscalls on open() - I 
noticed 2x fstat, 2x ioctl and 2x lseek. This is not noticable when working 
with small amounts of files on local filesystem, but if working with files via 
NSF or if working with huge amounts of files, lots of syscalls cost a lot of 
time. Therefore I'd like to create patches that would reduce the number of 
syscalls on open().
I've already managed to create first one (attached), that gets rid of one of 
the fstat calls (all the information are obtained from one fstat call).
I hope this makes sense and that the patch is acceptable. If not, I'll be happy 
to work on it to make it better. (This is my first real patch for C part of 
Python, so I hope I did everything right...)

--
components: IO
files: python3-remove-extraneous-fstat-on-file-open.patch
keywords: patch
messages: 219874
nosy: bkabrda
priority: normal
severity: normal
status: open
title: Prevent extraneous fstat during open()
type: resource usage
versions: Python 3.4, Python 3.5
Added file: 
http://bugs.python.org/file35494/python3-remove-extraneous-fstat-on-file-open.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21679] Prevent extraneous fstat during open()

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
nosy: +benjamin.peterson, pitrou, serhiy.storchaka, stutzbach

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21679
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com