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-117587: Add C implementation ofos.path.abspath#117855

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

Closed

Conversation

@nineteendo
Copy link
Contributor

@nineteendonineteendo commentedApr 13, 2024•
edited
Loading

Benchmark

posixpath.py by@eryksun:

absolute, with "/a" repeatedlen   speedup100   1.521x500   1.157x1000  1.128x2000  1.068x4000  1.019xrelative 'a', with cwd lengthlen     speedup93+2    1.861x548+2   1.729x1003+2  1.881x2043+2  1.556x4058+2  1.548x

ntpath.py

script
::speedup-posixpath.abspath.bat@echooffecho10 chars&&call main\python -m timeit -s"import os""os.path.abspath('a/' * 5)"&&call speedup-posixpath.abspath\python -m timeit -s"import os""os.path.abspath('a/' * 5)"echo100 chars&&call main\python -m timeit -s"import os""os.path.abspath('a/' * 50)"&&call speedup-posixpath.abspath\python -m timeit -s"import os""os.path.abspath('a/' * 50)"echo1000 chars&&call main\python -m timeit -s"import os""os.path.abspath('a/' * 500)"&&call speedup-posixpath.abspath\python -m timeit -s"import os""os.path.abspath('a/' * 500)"
10 chars500000 loops, best of 5: 635 nsec per loop # before500000 loops, best of 5: 517 nsec per loop # after# -> 1.23x faster100 chars200000 loops, best of 5: 1.53 usec per loop # before200000 loops, best of 5: 1.23 usec per loop # after# -> 1.24x faster1000 chars50000 loops, best of 5: 9.87 usec per loop # before50000 loops, best of 5: 7.73 usec per loop # after# -> 1.28x faster

@nineteendonineteendo marked this pull request as ready for reviewApril 13, 2024 20:03
@erlend-aaslanderlend-aasland changed the titlegh-117587: Speedup posixpath.abspathgh-117587: Add C implementation of posixpath.abspathApr 13, 2024
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
nineteendoand others added2 commitsApril 13, 2024 23:43
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@nineteendo
Copy link
ContributorAuthor

The same approach doesn't work on Window, becausentpath.join() is much slower thannt._getfullpathname():

PS C:\Users\wanne\cpython> python-m timeit-s"import nt""nt._getfullpathname('.')"; python-m timeit-s"import os""os.path.join(r'C:\Users\wanne\cpython', '')"1000000 loops, best of5:286 nsec per loop# _getfullpathname200000 loops, best of5:1.41 usec per loop# join

@eryksun

This comment was marked as resolved.

@nineteendo
Copy link
ContributorAuthor

Preferably,abspath() should not be naively implemented by simply joining a relative path with the working directory. It doesn't work for drive-relative paths. That's a long-standing bug in the generic implementationntpath._abspath_fallback().

I didn't even get that far in the testing process. I already knew it wasn't going to work.
I did notice this while trying to eliminatenormpath() from the current implementation.

@bedevere-app
Copy link

Thanks for making the requested changes!

@erlend-aasland: please review the changes made to this pull request.

@bedevere-app
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@nineteendo
Copy link
ContributorAuthor

erlend-aasland, I have made the requested changes; please review again.

@bedevere-app
Copy link

Thanks for making the requested changes!

@erlend-aasland: please review the changes made to this pull request.

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

Some nitpicks

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@eendebakpteendebakpteendebakpt left review comments

@eryksuneryksuneryksun approved these changes

@barneygalebarneygalebarneygale left review comments

@picnixzpicnixzpicnixz left review comments

@erlend-aaslanderlend-aaslanderlend-aasland requested changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nineteendo@eryksun@erlend-aasland@hugovk@eendebakpt@barneygale@picnixz
[8]ページ先頭
©2009-2025 Movatter.jp