Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-118761: Speedup pathlib import by deferring shutil#123520

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
barneygale merged 6 commits intopython:mainfromdanielhollas:speedup-pathlib
Sep 1, 2024

Conversation

@danielhollas
Copy link
Contributor

@danielhollasdanielhollas commentedAug 30, 2024
edited
Loading

~15% of pathlib import time (1.6ms on my machine) is spent on importingshutil. Since this module is only used in one place it makes sense to defer its import. Incidentally, this also makes the code more explicit imho, it took me a while to understand how the current code works.

image

barneygale reacted with thumbs up emoji
@hugovkhugovk changed the titlegh-118761: Speedup pathlib import by deffering shutilgh-118761: Speedup pathlib import by deferring shutilAug 31, 2024
Copy link
Member

@picnixzpicnixz left a comment
edited
Loading

Choose a reason for hiding this comment

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

On main in a non-debug build (but not a PGO one), I get no improvements at all.

The command to run is./python -m pyperf timeit 'import pathlib' -l 1 -w 0 -n 1 -p 250. This will ensure that you don't have warmups and don't have cached modules.

  • main:Mean +- std dev: 1.37 ms +- 0.05 ms
  • PR:Mean +- std dev: 1.36 ms +- 0.04 ms

I'm not sure this is worth the change but maybe I'm incorrectly benchmarking the time.

Some idea:shutil imports some additional libs such asfnmatch that are then needed byglob. So when you dofrom glob import _StringGlobber afterimport shutil, you'll end up importingfnmatch. So you save the fnmatch import in the glob module by importing shutil. Now I don't know if this specific import is the reason why I don't see an improvement.

@barneygale
Copy link
Contributor

I think this probably shouldn't have a NEWS entry, because theimport shutil was introduced only a couple of weeks ago, and there have been no releases from themain branch in that time. A user reading about a 15% performance improvement could be misled.

danielhollas reacted with thumbs up emoji

@danielhollas
Copy link
ContributorAuthor

The command to run is ./python -m pyperf timeit 'import pathlib' -l 1 -w 0 -n 1 -p 250. This will ensure that you don't have warmups and don't have cached modules.

1.6ms seems suspiciously fast, are you sure the module is not being cached?

I don't think fnmatch is the issue here, I think in the icicle graph I posted above it is already counted as part of glob. Most of the import time of shutil comes from importing compression modules, lzma etc. Measured withpython -Ximporttime -c 'import pathlib'. I think I used the PGO build, not sure what is the recommended build for benchmarking.

@hugovk
Copy link
Member

hugovk commentedAug 31, 2024
edited
Loading

I think I used the PGO build, not sure what is the recommended build for benchmarking.

Ideally usePGO+LTO.

On macOS with PGO+LTO, running./python.exe -X importtime -c "import pathlib", the pathlib import takes 7 ms withmain and 6 ms with this PR.

You can see the bigshutil block is missing underneathpathlib._local afterwards in the tuna importtime visualisations (I ran./python.exe -c "import pathlib" && ./python.exe -X importtime -c "import pathlib" 2> import.log && tuna import.log):

Before:

image

After:

image

And with hyperfine, this branch is about 1.5 ms faster thanmain:

hyperfine --warmup 32 \--prepare "git checkout speedup-pathlib"  './python.exe -c "import pathlib"' \--prepare "git checkout main"             './python.exe -c "import    pathlib"'Benchmark 1: ./python.exe -c "import pathlib"  Time (mean ± σ):      14.8 ms ±   0.6 ms    [User: 11.9 ms, System: 2.4 ms]  Range (min … max):    14.1 ms …  18.2 ms    96 runs  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs.Benchmark 2: ./python.exe -c "import    pathlib"  Time (mean ± σ):      16.4 ms ±   0.8 ms    [User: 13.1 ms, System: 2.7 ms]  Range (min … max):    15.6 ms …  23.2 ms    91 runs  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs.Summary  ./python.exe -c "import pathlib" ran    1.11 ± 0.07 times faster than ./python.exe -c "import    pathlib"
danielhollas reacted with heart emoji

danielhollasand others added2 commitsAugust 31, 2024 19:34
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@danielhollas
Copy link
ContributorAuthor

Thank you all for the reviews, I believe I addressed all the comments.

@picnixz
Copy link
Member

picnixz commentedSep 1, 2024
edited
Loading

1.6ms seems suspiciously fast, are you sure the module is not being cached?

With a caching it's in the realm of nanoseconds on my laptop. But my specs are a bit... biaised sometimes:

OS: openSUSE Leap 15.5 x86_64Host: ROG Strix G814JZ_G814JZ 1.0Kernel: 5.14.21-150500.55.73-defaultCPU: 13th Gen Intel i9-13980HX (32) @ 5.400GHzGPU: NVIDIA GeForce RTX 4080 Max-Q / MobileGPU: Intel Raptor Lake-S UHD GraphicsMemory: 4124MiB / 31698MiB

EDIT: It's a bit weird that pyperf reports timings in the realm of 1.6 ms but hyperfine does not:

Benchmark 1: ./python -c "import pathlib"  Time (mean ± σ):      11.5 ms ±   2.1 ms    [User: 9.7 ms, System: 1.9 ms]  Range (min … max):    10.2 ms …  20.8 ms    157 runs  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs.Benchmark 2: ./python -c "import    pathlib"  Time (mean ± σ):      11.9 ms ±   1.3 ms    [User: 10.5 ms, System: 1.4 ms]  Range (min … max):    11.0 ms …  21.7 ms    149 runs  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs.Summary  './python -c "import pathlib"' ran    1.03 ± 0.22 times faster than './python -c "import    pathlib"'

I'm approving the PR though it does not really change on my system much. But since it may affect other users, I think it's worth the change.

@barneygale
Copy link
Contributor

Thanks for this!

danielhollas reacted with heart emoji

@barneygalebarneygale merged commit2304774 intopython:mainSep 1, 2024
@danielhollasdanielhollas deleted the speedup-pathlib branchSeptember 1, 2024 14:46
@danielhollas
Copy link
ContributorAuthor

EDIT: It's a bit weird that pyperf reports timings in the realm of 1.6 ms but hyperfine does not:

hyperfine also measures the overhead of python startup.

Another possible explanation why you don't see a difference, do you have the compression modules available? You can trypython -c "import lzma; import zlib; import bz2

@picnixz
Copy link
Member

I do have them available :( it's an interesting observation though but it's my problem I think.

danielhollas reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@hugovkhugovkhugovk left review comments

@AA-TurnerAA-TurnerAA-Turner approved these changes

@picnixzpicnixzpicnixz approved these changes

@barneygalebarneygaleAwaiting requested review from barneygalebarneygale is a code owner

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@danielhollas@barneygale@hugovk@picnixz@AA-Turner

[8]ページ先頭

©2009-2025 Movatter.jp