Inefficient BufferedReader.read(-1)

[animalize]

BPO	41452
Nosy	@animalize
PRs	bpo-41452: BufferedReader.read(-1) no longer calls underlying stream's .readall() function #21698
Files	demo.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-08-01.05:59:43.634>
created_at = <Date 2020-08-01.03:34:14.219>
labels = ['3.10', 'expert-IO', 'performance']
title = 'Inefficient BufferedReader.read(-1)'
updated_at = <Date 2020-08-01.05:59:43.633>
user = 'https://github.com/animalize'

bugs.python.org fields:

activity = <Date 2020-08-01.05:59:43.633>
actor = 'malin'
assignee = 'none'
closed = True
closed_date = <Date 2020-08-01.05:59:43.634>
closer = 'malin'
components = ['IO']
creation = <Date 2020-08-01.03:34:14.219>
creator = 'malin'
dependencies = []
files = ['49354']
hgrepos = []
issue_num = 41452
keywords = ['patch']
message_count = 2.0
messages = ['374652', '374657']
nosy_count = 1.0
nosy_names = ['malin']
pr_nums = ['21698']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue41452'
versions = ['Python 3.10']

[animalize]

BufferedReader's constructor has a buffer_size parameter, it's the size of this buffer:

When reading data from BufferedReader object, a larger
amount of data may be requested from the underlying raw
stream, and kept in an internal buffer.

The doc of BufferedReader[1]

If call the BufferedReader.read(size) function:

1, When `size` is a positive number, it reads `buffer_size`
   bytes from the underlying stream. This is expected behavior.

2, When `size` is -1, it tries to call underlying stream's
   readall() function [2]. In this case `buffer_size` is not
   be respected.
   
   The underlying stream may be `RawIOBase`, its readall()
   function read `DEFAULT_BUFFER_SIZE` bytes in each read [3].
   
   `DEFAULT_BUFFER_SIZE` currently only 8KB, which is very
   inefficient for BufferedReader.read(-1). If `buffer_size`
   bytes is read every time, will be the expected performance.

Attached file demonstrates this problem.

[1] doc of BufferedReader:
https://docs.python.org/3/library/io.html#io.BufferedReader

[2] BufferedReader.read(-1) tries to call underlying stream's readall() function:
https://github.com/python/cpython/blob/v3.9.0b5/Modules/_io/bufferedio.c#L1538-L1542

[3] RawIOBase.readall() read DEFAULT_BUFFER_SIZE each time:
https://github.com/python/cpython/blob/v3.9.0b5/Modules/_io/iobase.c#L968-L969

[animalize]

Some underlying stream has fast-path for .readall().
So close this issue.

[mxmlnkn]

I have the same issue. It is hugely confusing that the buffer size is ignored for read(-1). And regarding it being a "fast-path", it is not. It is 100 times slower on my Lustre test filesystem, which has a block size of 4 MiB. I think the issue is the implementation of readall in RawIOBase, from which I inherited. Maybe that readall implementation could be removed as I don't see it adding any value. On the other hand, maybe I am wrong and shouldn't inherit from RawIOBase, but then what should I inherit from when implementing my own I/O classes?