Created on 2018-06-30 09:27 by hajoscher, last changed 2018-07-04 12:27 by vstinner. This issue is now closed.
Buffer read of large files in a compressed tarfile stream performs poorly.
The buffered read in tarfile _Stream is extending a bytes object.
It is much more efficient to use a list followed by a join.
Using a list can mean seconds instead of minutes.
This performance regression was introduced in b506dc32c1a.
How to test:
# create random tarfile 50Mb
dd if=/dev/urandom of=test.bin count=50 bs=1M
tar czvf test.tgz test.bin
# read with tarfile as stream (note pipe symbol in 'r|gz')
import tarfile
tfile = tarfile.open("test.tgz", 'r|gz')
for t in tfile:
file = tfile.extractfile(t)
if file:
print(len(file.read()))
Nice catch.
I confirmed this is a hard regression of performance.
Decompressing a file must be O(n) when n=filesize, but O(n^2) now.
While we live with this regression for a long time, I feel it's worth enough to backport.
This can be DoS vulnerability.
Can you write NEWS entry for it?
Yes, it performance is really bad for large files, and memory consumption as well. I will write something for NEWS.
New changeset 12a08c47601cadea8e7d3808502cdbcca87b2ce2 by INADA Naoki (hajoscher) in branch 'master':
bpo-34010: Fix tarfile read performance regression (GH-8020)
thanks
- buf = "".join(t)
+ buf = b"".join(t)
@hajoscher: "It never caused a problem, since this line is never called; size is never None in the function call. But still, should be fixed, I guess."
Would it be possible to have an unit test for this modified line? Untested code is broken, as you showed :-)
New changeset c1b75b5fb92fda0ac5b931d7b18c1418557cb7c4 by Miss Islington (bot) in branch '3.7':
bpo-34010: Fix tarfile read performance regression (GH-8020)
New changeset d7a0ad7dd7bd7dfbdbf6be2c89fde5a71813628a by Miss Islington (bot) in branch '3.6':
bpo-34010: Fix tarfile read performance regression (GH-8020)
@Victor I think removing unused code is better than adding test for it and maintain it.
So I removed that unused code block in GH-8089.
> @Victor I think removing unused code is better than adding test for it and maintain it.
Sure. I reviewed your PR 8089.