[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for contribution.

--
resolution:  -> fixed
stage: patch review -> committed/rejected
status: open -> closed

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Roundup Robot]

Roundup Robot added the comment:

New changeset 088a14031998 by Serhiy Storchaka in branch 'default':
Issue #8745: Small speed up zipimport on Windows. Patch by Catalin Iacob.
http://hg.python.org/cpython/rev/088a14031998

--
nosy: +python-dev

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Sorry, Brian. Raymond is an initiator of issue17004.

--
nosy: +rhettinger -brian.curtin

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +brian.curtin

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

In general, the patch LGTM, however I can't try it on Windows, and on Linux it 
has no any performance effect. Can anyone try it on Windows?

I have re-uploaded the patch for review after converting it from UTF-16 and 
CRLF.

--
Added file: http://bugs.python.org/file28612/zipimport-speedup-v4.patch

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Catalin Iacob added the comment:

Attached v4 of patch with error checking for getc and some more comments.

A real world example of the speedup is Calibre (http://calibre-ebook.com/) 
which on Windows comes with its own code, its dependecies and Python's stdlib 
all bundled in a 40MB zip. With the patch the time to create a zipimporter 
instance from that zip drops from around 60ms to around 15ms.

--
Added file: http://bugs.python.org/file28599/zipimport-speedup-v4.patch

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Catalin Iacob added the comment:

Yes Serhiy, I was planning to, lack of time followed by (just finished) 
vacation lead to a stall. Thanks for following up on this.

I should soon, probably this weekend, have an updated version addressing your 
comments.

--

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Catalin, are you going to continue?

--

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> So this patch is a workaround for poor fseek performance in Microsoft's CRT, 
> it doesn't cause performance issues on Linux but saves quite some 
> milliseconds of startup time so I think it's worth the tradeoff.

I think some clarifying comments will be good in the code. In particular about 
the `dummy` variable.

> I'll also upload zipimport_speedup-v3.patch updated to apply to current 
>default and with error handling for failing freads since the fseeks that the 
>patch replaces have gained error handling on the default branch in the mean 
>time.

Perhaps getc() requires error handling too.

--

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Changes by Catalin Iacob :


Added file: http://bugs.python.org/file28117/zipimport-speedup-v3.patch

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Catalin Iacob added the comment:

I tried Serhiy's suggestion in msg174934, see attached 
attempt-fseek-seek_cur.patch updated to current default. It makes no difference 
relative to the default branch for my test stdlib.zip, dummy reads still work 
better on Windows.

This makes sense if you follow the CRT's implementation, fseek calls 
_fseek_nolock which always calls _flush, regardless whether SEEK_CUR is used or 
not. This is the case with both VS2008 and VS2010. So this patch is a 
workaround for poor fseek performance in Microsoft's CRT, it doesn't cause 
performance issues on Linux but saves quite some milliseconds of startup time 
so I think it's worth the tradeoff.

I'll also upload zipimport_speedup-v3.patch updated to apply to current default 
and with error handling for failing freads since the fseeks that the patch 
replaces have gained error handling on the default branch in the mean time. The 
timings remain the same on my Windows machine: around 30ms for default branch, 
around 15ms with patch.

--
Added file: http://bugs.python.org/file28116/attempt-fseek-seek_cur.patch

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Eric Snow]

Changes by Eric Snow :


--
nosy: +eric.snow

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I suggest to use fseek(fp, relative_offset, SEEK_CUR).  It doesn't force a 
system call (at least on Linux) and may be a little faster than fread() or 
multiple getc().

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Changes by Catalin Iacob :


--
versions: +Python 3.4 -Python 3.3

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Catalin Iacob]

Catalin Iacob  added the comment:

I updated Goplat's patch to the default branch.

It now needs to read 4 dummy bytes instead of 6 since an extra 
PyMarshal_ReadShortFromFile was added to the default branch in the mean time. I 
added an explicit dummy buffer instead of reading the dummy bytes into name 
(for cleanness and because name would overflow on hypothetical platforms where 
MAXPATHLEN + 5 < 8). Also added tests for the loop that skips the rest of the 
header by creating some zips with file comments; without the extra test, 
commenting out the loop didn't fail test_zipimport.

Running Goplat's test in msg106191 on Windows I get 0.032 sec before and 0.015 
sec after. On Linux I see no significant difference.

AFAIK Mercurial (for example) ships with a zipped stdlib on Windows and they 
care quite a lot about startup time. Can this make it into 3.3?

--
nosy: +catalin.iacob
versions: +Python 3.3 -Python 3.2
Added file: http://bugs.python.org/file25823/zipimport_speedup-v2.patch

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Brett Cannon]

Changes by Brett Cannon :


--
assignee: brett.cannon -> 

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Goplat]

Goplat  added the comment:

Zipping up the Lib directory from the python source (1735 files) as a test, it 
took on average 0.10 sec to read the zip before, 0.04 sec after.

(To test the time taken by zipimport isolated from other startup costs, instead 
of actually getting the zip in the import path, I just ran

import time, zipimport; start = time.clock(); 
zipimport.zipimporter("lib.zip"); print time.clock() - start)

--

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
assignee:  -> brett.cannon
nosy: +brett.cannon
stage:  -> patch review
versions: +Python 3.2 -Python 2.6

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Ray.Allen]

Ray.Allen  added the comment:

When I perform some test on debian-5.0, I see the timing results almost the 
same before and after apply your patch(I modified the patch to against the 
trunk). 

Could you give some test result on windows? I can't see the speedups on 
debian-5.0.

--
nosy: +ysj.ray

___
Python tracker 

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

[issue8745] zipimport is a bit slow

[Goplat]

New submission from Goplat :

Reading the list of files in a .zip takes a while because several seeks are 
done for each entry, which (on Windows at least) flushes stdio's buffer and 
forces a system call on the next read. For large .zips the effect on startup 
speed is noticeable, being perhaps 50ms per thousand files. Changing the 
read_directory function to read the central directory entirely sequentially 
would cut this time by more than half.

--
components: Interpreter Core
files: zipimport_speedup.patch
keywords: patch
messages: 105954
nosy: Goplat
priority: normal
severity: normal
status: open
title: zipimport is a bit slow
type: performance
versions: Python 2.6
Added file: http://bugs.python.org/file17387/zipimport_speedup.patch

___
Python tracker 

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