Skip to content
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

gh-118761: improve import time for secrets #128738

Closed
wants to merge 2 commits into from

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Jan 11, 2025

Benchmarks were performed on a RELEASE build (no PGO, no LTO).

PR

$ ./python -X importtime -c 'import secrets'
import time: self [us] | cumulative | imported package
...
import time:       236 |        236 | linecache
import time:       415 |        415 |     warnings
import time:       698 |        698 |     _hashlib
import time:       133 |        133 |       _blake2
import time:       226 |        358 |     hashlib
import time:       498 |       1968 |   hmac
import time:       152 |        152 |     math
import time:        66 |         66 |       _operator
import time:       252 |        318 |     operator
import time:        97 |         97 |     itertools
import time:        79 |         79 |       _bisect
import time:        99 |        177 |     bisect
import time:        70 |         70 |     _random
import time:       418 |       1230 |   random
import time:       139 |       3336 | secrets
$ hyperfine --warmup 16 "./python -c 'import secrets'"
Benchmark 1: ./python -c 'import secrets'
  Time (mean ± σ):       7.0 ms ±   0.7 ms    [User: 6.0 ms, System: 1.2 ms]
  Range (min … max):     6.5 ms …  14.9 ms    372 runs

Main

$ ./python -X importtime -c 'import secrets'
import time: self [us] | cumulative | imported package
...
import time:       190 |        190 | linecache
import time:       243 |        243 |         types
import time:      1168 |       1411 |       enum
import time:        58 |         58 |         _sre
import time:       187 |        187 |           re._constants
import time:       269 |        455 |         re._parser
import time:        67 |         67 |         re._casefix
import time:       240 |        819 |       re._compiler
import time:        71 |         71 |           itertools
import time:        72 |         72 |           keyword
import time:        41 |         41 |             _operator
import time:       163 |        203 |           operator
import time:       101 |        101 |           reprlib
import time:        40 |         40 |           _collections
import time:       603 |       1089 |         collections
import time:        36 |         36 |         _functools
import time:       423 |       1548 |       functools
import time:       105 |        105 |       copyreg
import time:       403 |       4285 |     re
import time:       100 |        100 |       _struct
import time:        67 |        166 |     struct
import time:       113 |        113 |     binascii
import time:       387 |       4949 |   base64
import time:       157 |        157 |     warnings
import time:       297 |        297 |     _hashlib
import time:        67 |         67 |       _blake2
import time:       113 |        179 |     hashlib
import time:       113 |        744 |   hmac
import time:        75 |         75 |     math
import time:        39 |         39 |       _bisect
import time:        51 |         90 |     bisect
import time:        37 |         37 |     _random
import time:       335 |        535 |   random
import time:       105 |       6332 | secrets
$ hyperfine --warmup 16 "./python -c 'import secrets'"
Benchmark 1: ./python -c 'import secrets'
  Time (mean ± σ):      10.9 ms ±   0.6 ms    [User: 9.3 ms, System: 1.5 ms]
  Range (min … max):    10.2 ms …  14.7 ms    262 runs

Importing `secrets` is now two times faster.

The `base64` module is now locally imported by `secrets.token_urlsafe`
and is no more implicitly exposed as `secrets.base64`.
@AA-Turner
Copy link
Member

We should re-evaluate this after base64 no longer imports re.

@picnixz
Copy link
Member Author

picnixz commented Jan 12, 2025

Oh yes, you're right. I just uploaded the PRs I had locally and forgot about the dependency.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@picnixz
Copy link
Member Author

picnixz commented Jan 14, 2025

Since base64 will not import re anymore, we can perhaps avoid this change as I fear it won't be impactful at all. So I plan to defer this one until #128736 is merged (and re-evaluate the improvements). If there's no improvement, we can just keep the import as it is.

@picnixz picnixz marked this pull request as draft January 14, 2025 13:00
@vstinner
Copy link
Member

#128736 was merged.

@picnixz
Copy link
Member Author

picnixz commented Jan 14, 2025

I'm actually no more on my dev session so if someone wants to check how long import secrets now takes, I would be happy. Otherwise, I'll do it tomorrow.

@hugovk
Copy link
Member

hugovk commented Jan 14, 2025

On macOS M2 with release+PGO+LTO.

PR with main merged

./python.exe -X importtime -c 'import secrets'
import time: self [us] | cumulative | imported package
import time:       238 |        238 |   _io
import time:        31 |         31 |   marshal
import time:       324 |        324 |   posix
import time:       698 |       1290 | _frozen_importlib_external
import time:       687 |        687 |   time
import time:       255 |        942 | zipimport
import time:        59 |         59 |     _codecs
import time:       655 |        714 |   codecs
import time:       504 |        504 |   encodings.aliases
import time:       903 |       2120 | encodings
import time:       211 |        211 | encodings.utf_8
import time:        78 |         78 | _signal
import time:        37 |         37 |     _abc
import time:       172 |        208 |   abc
import time:       182 |        390 | io
import time:        43 |         43 |       _stat
import time:        86 |        128 |     stat
import time:       832 |        832 |     _collections_abc
import time:        75 |         75 |       errno
import time:        71 |         71 |       genericpath
import time:       199 |        345 |     posixpath
import time:       578 |       1881 |   os
import time:       103 |        103 |   _sitebuiltins
import time:       238 |        238 |   encodings.utf_8_sig
import time:       569 |        569 |   types
import time:       156 |        156 |     importlib
import time:       211 |        211 |     importlib._abc
import time:       151 |        517 |   importlib.util
import time:        42 |         42 |   importlib.machinery
import time:       399 |        399 |   sitecustomize
import time:        49 |         49 |   usercustomize
import time:      1899 |       5692 | site
import time:       409 |        409 | linecache
import time:       474 |        474 |     warnings
import time:      2950 |       2950 |     _hashlib
import time:       494 |        494 |       _blake2
import time:       599 |       1092 |     hashlib
import time:       542 |       5057 |   hmac
import time:      1134 |       1134 |     math
import time:        76 |         76 |       _operator
import time:       330 |        406 |     operator
import time:        86 |         86 |     itertools
import time:       436 |        436 |       _bisect
import time:       316 |        752 |     bisect
import time:       365 |        365 |     _random
import time:       742 |       3483 |   random
import time:      2877 |      11416 | secrets
hyperfine --warmup 16 "./python.exe -c 'import secrets'"
Benchmark 1: ./python.exe -c 'import secrets'
  Time (mean ± σ):      14.7 ms ±   1.0 ms    [User: 10.9 ms, System: 3.1 ms]
  Range (min … max):    13.2 ms …  24.5 ms    164 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

main

./python.exe -X importtime -c 'import secrets'
import time: self [us] | cumulative | imported package
import time:       200 |        200 |   _io
import time:        31 |         31 |   marshal
import time:       265 |        265 |   posix
import time:       585 |       1079 | _frozen_importlib_external
import time:       565 |        565 |   time
import time:       237 |        802 | zipimport
import time:        49 |         49 |     _codecs
import time:       541 |        590 |   codecs
import time:       504 |        504 |   encodings.aliases
import time:       833 |       1926 | encodings
import time:       222 |        222 | encodings.utf_8
import time:        77 |         77 | _signal
import time:        31 |         31 |     _abc
import time:       158 |        188 |   abc
import time:       170 |        358 | io
import time:        53 |         53 |       _stat
import time:        99 |        152 |     stat
import time:       831 |        831 |     _collections_abc
import time:        56 |         56 |       errno
import time:        59 |         59 |       genericpath
import time:       132 |        246 |     posixpath
import time:       525 |       1752 |   os
import time:        85 |         85 |   _sitebuiltins
import time:       290 |        290 |   encodings.utf_8_sig
import time:       578 |        578 |   types
import time:       168 |        168 |     importlib
import time:       222 |        222 |     importlib._abc
import time:       172 |        560 |   importlib.util
import time:        45 |         45 |   importlib.machinery
import time:       449 |        449 |   sitecustomize
import time:        53 |         53 |   usercustomize
import time:      2183 |       5991 | site
import time:       488 |        488 | linecache
import time:       564 |        564 |       _struct
import time:       124 |        688 |     struct
import time:       381 |        381 |     binascii
import time:       251 |       1319 |   base64
import time:       471 |        471 |     warnings
import time:      3136 |       3136 |     _hashlib
import time:       807 |        807 |       _blake2
import time:       788 |       1594 |     hashlib
import time:       518 |       5717 |   hmac
import time:       610 |        610 |     math
import time:        68 |         68 |       _operator
import time:       317 |        385 |     operator
import time:        73 |         73 |     itertools
import time:       387 |        387 |       _bisect
import time:       461 |        847 |     bisect
import time:       334 |        334 |     _random
import time:       873 |       3121 |   random
import time:      2587 |      12742 | secrets
hyperfine --warmup 16 "./python.exe -c 'import secrets'"
Benchmark 1: ./python.exe -c 'import secrets'
  Time (mean ± σ):      15.4 ms ±   1.1 ms    [User: 11.3 ms, System: 3.4 ms]
  Range (min … max):    14.1 ms …  26.0 ms    168 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

@picnixz
Copy link
Member Author

picnixz commented Jan 14, 2025

I therefore don't think it's worth the change. Less work for me and less diff with other branches. I'm going to close this PR.

@picnixz picnixz closed this Jan 14, 2025
@picnixz
Copy link
Member Author

picnixz commented Jan 14, 2025

Thank you Hugo for running the numbers. There is still one module that I had in mind (pickletools) since it imports re but I haven't checked other modules importing re. I'll do that tomorrow if I time, otherwise this w-e.

@picnixz picnixz deleted the perf/import/secrets-118761 branch January 14, 2025 17:37
@vstinner
Copy link
Member

I don't think that pickletools import time matters since pickletools is a debug tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants