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-117686: Improve the performance of ntpath.expanduser() #117690

Merged
merged 6 commits into from
Apr 10, 2024

Conversation

nineteendo
Copy link
Contributor

@nineteendo nineteendo commented Apr 9, 2024

Benchmark

script
::test.bat
@echo off
echo 1 character && python -m timeit -s "import before.ntpath" "before.ntpath.expanduser('~a')" && python -m timeit -s "import after.ntpath" "after.ntpath.expanduser('~a')"
echo 10 characters && python -m timeit -s "import before.ntpath" "before.ntpath.expanduser('~' + 'a' * 10)" && python -m timeit -s "import after.ntpath" "after.ntpath.expanduser('~' + 'a' * 10)"
echo 100 characters && python -m timeit -s "import before.ntpath" "before.ntpath.expanduser('~' + 'a' * 100)" && python -m timeit -s "import after.ntpath" "after.ntpath.expanduser('~' + 'a' * 100)"
1 character
50000 loops, best of 5: 5.93 usec per loop # before
50000 loops, best of 5: 5.77 usec per loop # after
# -> 1.03x faster
10 characters
50000 loops, best of 5: 7.49 usec per loop # before
50000 loops, best of 5: 6.43 usec per loop # after
# -> 1.16x faster
100 characters
20000 loops, best of 5: 19.5 usec per loop # before
20000 loops, best of 5: 12.2 usec per loop # after
# -> 1.60x faster

@nineteendo nineteendo marked this pull request as ready for review April 9, 2024 18:22
@erlend-aasland erlend-aasland changed the title gh-117686: Move _get_bothseps() outside loop condition gh-117686: Improve the performance of ntpath.expanduser() Apr 9, 2024
Lib/ntpath.py Outdated Show resolved Hide resolved
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

I think this change is fine; it is small and non-intrusive, and readability is not impacted. Still, let's wait a day or two before merging in case anyone disagrees.

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Apr 10, 2024

Thumbs up from Terry in #117686 (comment).

With two core devs being in favour, I'll go ahead and merge.

@erlend-aasland erlend-aasland merged commit f90ff03 into python:main Apr 10, 2024
36 checks passed
@nineteendo nineteendo deleted the speedup-ntpath.expanduser branch April 10, 2024 08:42
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants