-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove import copyreg from os module #63408
Comments
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. |
+1 |
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? |
>>> 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) |
What will happen when do not register stat_result and statvfs_result at all? |
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
... |
Can't we modify the qualified name instead? |
But for pickling something, you have to import pickle, which always imports copyreg anyway, doesn't it? |
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. |
I don't see a problem with that. |
How much this speed up the startup of the interpreter? Proposed patch looks contrary to purpose of the copyreg module. |
The speedup is minimal but it's a start. |
os_no_copyreg.patch looks good to me. |
I don't think we should tangent code for such tiny benefit. copyreg is lightweight module specially designed to break coupling of the code. |
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) |
os_stat_statvfs_pickle.patch with comments and tests. |
New changeset 29c4a6a11e76 by Christian Heimes in branch 'default': |
Thanks for your help! Python is down to 43 modules on Linux. |
New changeset 89e405e6a7a9 by Christian Heimes in branch 'default': |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: