Issue19209
Created on 2013-10-09 14:21 by christian.heimes, last changed 2022-04-11 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| os_no_copyreg.patch | christian.heimes, 2013-10-09 14:21 | review | ||
| os_stat_statvfs_pickle.patch | vstinner, 2013-10-10 08:09 | review | ||
| os_stat_statvfs_pickle2.patch | christian.heimes, 2013-10-11 22:53 | review | ||
| Messages (19) | |||
|---|---|---|---|
| msg199303 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-09 14:21 | |
The patch removes "import copyreg" from the os module and moves the registration of the hooks to copyreg. This speeds up the startup of the interpreter a tiny bit. |
|||
| msg199304 - (view) | Author: Georg Brandl (georg.brandl) * ![]() |
Date: 2013-10-09 14:28 | |
+1 |
|||
| msg199305 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-10-09 15:29 | |
I don't know the copyreg module! Does it have a unit test for the registered os objects? If not, how can it be tested manually? |
|||
| msg199306 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-09 15:33 | |
>>> import os
>>> import pickle
>>> pickle.dumps(os.stat("."))
b"\x80\x03cos\n_make_stat_result\nq\x00(M\xfdAJ\x84\xa0k\x00M\x00\xfcK\x13M\xe8\x03M\xe8\x03M\x00`J\x9chURJ\xbdgURJ\xbdgURtq\x01}q\x02(X\x08\x00\x00\x00st_ctimeq\x03GA\xd4\x95Y\xefW\x04\xc1X\x07\x00\x00\x00st_rdevq\x04K\x00X\x0b\x00\x00\x00st_mtime_nsq\x05\x8a\x08\xe8/\xfdo@w+\x13X\x08\x00\x00\x00st_atimeq\x06GA\xd4\x95Z'$T\xb6X\x0b\x00\x00\x00st_ctime_nsq\x07\x8a\x08\xe8/\xfdo@w+\x13X\n\x00\x00\x00st_blksizeq\x08M\x00\x10X\x0b\x00\x00\x00st_atime_nsq\t\x8a\x08\xa0\x0e9htw+\x13X\x08\x00\x00\x00st_mtimeq\nGA\xd4\x95Y\xefW\x04\xc1X\t\x00\x00\x00st_blocksq\x0bK8u\x86q\x0cRq\r."
>>> pickle.loads(pickle.dumps(os.stat(".")))
posix.stat_result(st_mode=16893, st_ino=7053444, st_dev=64512, st_nlink=19, st_uid=1000, st_gid=1000, st_size=24576, st_atime=1381329052, st_mtime=1381328829, st_ctime=1381328829)
|
|||
| msg199307 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-10-09 15:46 | |
What will happen when do not register stat_result and statvfs_result at all? |
|||
| msg199309 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-09 16:47 | |
You can still pickle and unpickle the objects but the result is no longer platform-independent as it refers to "posix" or "nt" instead of "os".
>>> import os, pickle, pickletools
>>> pickletools.dis(pickle.dumps(os.stat(".")))
0: \x80 PROTO 3
2: c GLOBAL 'os _make_stat_result'
24: q BINPUT 0
26: ( MARK
...
>>> pickletools.dis(pickle.dumps(os.stat(".")))
0: \x80 PROTO 3
2: c GLOBAL 'posix stat_result'
21: q BINPUT 0
23: ( MARK
...
|
|||
| msg199322 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-10-09 18:25 | |
Can't we modify the qualified name instead? |
|||
| msg199326 - (view) | Author: Georg Brandl (georg.brandl) * ![]() |
Date: 2013-10-09 18:31 | |
But for pickling something, you have to import pickle, which always imports copyreg anyway, doesn't it? |
|||
| msg199327 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-09 18:33 | |
Exactly, the pickle module depends on the copyreg module. It's a submodule that acts as a registry for pickle-related lookups and hooks. My patch just moves the registration of these hooks out of the os module into the copyreg module. |
|||
| msg199328 - (view) | Author: Georg Brandl (georg.brandl) * ![]() |
Date: 2013-10-09 18:35 | |
I don't see a problem with that. |
|||
| msg199329 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-10-09 18:38 | |
How much this speed up the startup of the interpreter? Proposed patch looks contrary to purpose of the copyreg module. |
|||
| msg199333 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-09 19:21 | |
The speedup is minimal but it's a start. |
|||
| msg199334 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-10-09 19:25 | |
os_no_copyreg.patch looks good to me. |
|||
| msg199336 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2013-10-09 19:34 | |
I don't think we should tangent code for such tiny benefit. copyreg is lightweight module specially designed to break coupling of the code. |
|||
| msg199369 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2013-10-10 08:09 | |
> Can't we modify the qualified name instead?
Attached os_stat_statvfs_pickle.patch implements this idea. IMO it's much simpler because it removes completly the need of the copyreg module.
Example with the patch on Linux:
$ ./python
Python 3.4.0a3+ (default:63a1ee94b3ed+, Oct 10 2013, 10:03:45)
>>> import os, pickletools, pickle
>>> s=os.stat('.')
>>> pickletools.dis(pickle.dumps(s))
0: \x80 PROTO 3
2: c GLOBAL 'os stat_result'
...
>>> pickle.loads(pickle.dumps(s))
os.stat_result(st_mode=16893, st_ino=19792207, st_dev=64772, st_nlink=17, st_uid=1000, st_gid=1000, st_size=28672, st_atime=1381392226, st_mtime=1381392226, st_ctime=1381392226)
>>>
>>> v=os.statvfs('.')
>>> pickletools.dis(pickle.dumps(v))
0: \x80 PROTO 3
2: c GLOBAL 'os statvfs_result'
...
>>> pickle.loads(pickle.dumps(v))
os.statvfs_result(f_bsize=4096, f_frsize=4096, f_blocks=125958458, f_bfree=124095595, f_bavail=117695595, f_files=32006144, f_ffree=31792079, f_favail=31792079, f_flag=4096, f_namemax=255)
|
|||
| msg199517 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-11 22:53 | |
os_stat_statvfs_pickle.patch with comments and tests. |
|||
| msg199522 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-10-11 23:27 | |
New changeset 29c4a6a11e76 by Christian Heimes in branch 'default': Issue #19209: Remove import of copyreg from the os module to speed up http://hg.python.org/cpython/rev/29c4a6a11e76 |
|||
| msg199523 - (view) | Author: Christian Heimes (christian.heimes) * ![]() |
Date: 2013-10-11 23:29 | |
Thanks for your help! Python is down to 43 modules on Linux. |
|||
| msg199524 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2013-10-11 23:42 | |
New changeset 89e405e6a7a9 by Christian Heimes in branch 'default': Issue #19209: fix structseq test http://hg.python.org/cpython/rev/89e405e6a7a9 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:51 | admin | set | github: 63408 |
| 2013-10-11 23:42:01 | python-dev | set | messages: + msg199524 |
| 2013-10-11 23:29:43 | christian.heimes | set | status: open -> closed resolution: fixed messages: + msg199523 stage: patch review -> resolved |
| 2013-10-11 23:27:40 | python-dev | set | nosy:
+ python-dev messages: + msg199522 |
| 2013-10-11 22:53:01 | christian.heimes | set | files:
+ os_stat_statvfs_pickle2.patch messages: + msg199517 |
| 2013-10-10 08:09:22 | vstinner | set | files:
+ os_stat_statvfs_pickle.patch messages: + msg199369 |
| 2013-10-09 19:34:55 | serhiy.storchaka | set | messages: + msg199336 |
| 2013-10-09 19:25:53 | vstinner | set | messages: + msg199334 |
| 2013-10-09 19:21:31 | christian.heimes | set | messages: + msg199333 |
| 2013-10-09 18:38:02 | serhiy.storchaka | set | messages: + msg199329 |
| 2013-10-09 18:35:25 | georg.brandl | set | messages: + msg199328 |
| 2013-10-09 18:33:45 | christian.heimes | set | messages: + msg199327 |
| 2013-10-09 18:31:15 | georg.brandl | set | messages: + msg199326 |
| 2013-10-09 18:25:46 | vstinner | set | messages: + msg199322 |
| 2013-10-09 17:17:35 | barry | set | nosy:
+ barry |
| 2013-10-09 16:47:46 | christian.heimes | set | messages: + msg199309 |
| 2013-10-09 15:46:01 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg199307 |
| 2013-10-09 15:33:41 | christian.heimes | set | messages: + msg199306 |
| 2013-10-09 15:29:51 | vstinner | set | nosy:
+ vstinner messages: + msg199305 |
| 2013-10-09 14:28:29 | georg.brandl | set | nosy:
+ georg.brandl messages: + msg199304 |
| 2013-10-09 14:21:20 | christian.heimes | create | |

