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-117349: Speedup os.path #117610

Closed
wants to merge 18 commits into from
Closed

Conversation

nineteendo
Copy link
Contributor

@nineteendo nineteendo commented Apr 7, 2024

Benchmark:

  • ntpath.py

    ::test.bat
    @echo off
    echo join() && python -m timeit -s "import before.ntpath" "before.ntpath.join('foo')" && python -m timeit -s "import after.ntpath" "after.ntpath.join('foo')"
    echo split() && python -m timeit -s "import before.ntpath" "before.ntpath.split('foo')" && python -m timeit -s "import after.ntpath" "after.ntpath.split('foo')"
    echo ismount() && python -m timeit -s "import before.ntpath" "before.ntpath.ismount('//')" && python -m timeit -s "import after.ntpath" "after.ntpath.ismount('//')"
    echo expanduser() && python -m timeit -s "import before.ntpath" "before.ntpath.expanduser('~')" && python -m timeit -s "import after.ntpath" "after.ntpath.expanduser('~')"
    echo expanduser('~' + 'a' * 100) && python -m timeit -s "import before.ntpath" "before.ntpath.expanduser('~' + 'a' * 100)" && python -m timeit -s "import after.ntpath" "after.ntpath.expanduser('~' + 'a' * 100)"
    echo realpath(): cwd && python -m timeit -s "import os" "cwd = os.getcwd()" && python -m timeit -s "import os" "getcwd = os.getcwd; cwd = getcwd()"
    echo realpath('C:/'): cwd && python -m timeit -s "import os" "cwd = os.getcwd()" && python -m timeit -s "import os" "getcwd = os.getcwd"
    join()
    500000 loops, best of 5: 693 nsec per loop # before
    500000 loops, best of 5: 650 nsec per loop # after
    # -> 1.07x faster
    split()
    500000 loops, best of 5: 866 nsec per loop # before
    500000 loops, best of 5: 799 nsec per loop # after
    # -> 1.08x faster
    ismount()
    200000 loops, best of 5: 1.19 usec per loop # before
    200000 loops, best of 5: 1.13 usec per loop # after
    # -> 1.05x faster
    expanduser()
    500000 loops, best of 5: 973 nsec per loop # before
    500000 loops, best of 5: 967 nsec per loop # after
    # -> no difference
    expanduser('~' + 'a' * 100)
    20000 loops, best of 5: 18 usec per loop # before
    20000 loops, best of 5: 11.2 usec per loop # after
    # -> 1.61x faster (for long users)
    realpath(): cwd 
    1000000 loops, best of 5: 228 nsec per loop # before
    1000000 loops, best of 5: 236 nsec per loop # after
    # -> only 8 nsec slower
    realpath('C:/'): cwd 
    1000000 loops, best of 5: 232 nsec per loop # before
    20000000 loops, best of 5: 16.3 nsec per loop # after
    # -> always 16.3 nsec (for absolute paths)
    
  • posixpath.py

    # test.sh
    echo "isabs()" && python -m timeit -s "import before.posixpath" "before.posixpath.isabs('/')" && python -m timeit -s "import after.posixpath" "after.posixpath.isabs('/')"
    echo "join()" && python -m timeit -s "import before.posixpath" "before.posixpath.join('foo')" && python -m timeit -s "import after.posixpath" "after.posixpath.join('foo')"
    echo "split()" && python -m timeit -s "import before.posixpath" "before.posixpath.split('foo')" && python -m timeit -s "import after.posixpath" "after.posixpath.split('foo')"
    echo "basename()" && python -m timeit -s "import before.posixpath" "before.posixpath.basename('foo')" && python -m timeit -s "import after.posixpath" "after.posixpath.basename('foo')"
    echo "dirname()" && python -m timeit -s "import before.posixpath" "before.posixpath.dirname('foo')" && python -m timeit -s "import after.posixpath" "after.posixpath.dirname('foo')"
    echo "expanduser()" && python -m timeit -s "import before.posixpath" "before.posixpath.expanduser('~')" && python -m timeit -s "import after.posixpath" "after.posixpath.expanduser('~')"
    echo "abspath()" && python -m timeit -s "import before.posixpath" "before.posixpath.abspath('foo')" && python -m timeit -s "import after.posixpath" "after.posixpath.abspath('foo')"
    echo "abspath('.')" && python -m timeit -s "import before.posixpath" "before.posixpath.abspath('.')" && python -m timeit -s "import after.posixpath" "after.posixpath.abspath('.')"
    echo "commonpath()" && python -m timeit -s "import before.posixpath" "before.posixpath.commonpath(['/'])" && python -m timeit -s "import after.posixpath" "after.posixpath.commonpath(['/'])"
    isabs()
    1000000 loops, best of 5: 230 nsec per loop # before
    1000000 loops, best of 5: 207 nsec per loop # after
    # -> 1.11x faster
    join()
    1000000 loops, best of 5: 322 nsec per loop # before
    1000000 loops, best of 5: 244 nsec per loop # after
    # -> 1.32x faster
    split()
    1000000 loops, best of 5: 348 nsec per loop # before
    1000000 loops, best of 5: 323 nsec per loop # after
    # -> 1.08x faster
    basename()
    1000000 loops, best of 5: 285 nsec per loop # before
    1000000 loops, best of 5: 263 nsec per loop # after
    # -> 1.08x faster
    dirname()
    1000000 loops, best of 5: 290 nsec per loop # before
    1000000 loops, best of 5: 259 nsec per loop # after
    # -> 1.12x faster
    expanduser()
    200000 loops, best of 5: 1.35 usec per loop # before
    200000 loops, best of 5: 1.25 usec per loop # after
    # -> 1.08x faster
    abspath()
    20000 loops, best of 5: 16.7 usec per loop # before
    20000 loops, best of 5: 16.8 usec per loop # after
    # -> no difference
    abspath('.')
    20000 loops, best of 5: 17 usec per loop # before
    20000 loops, best of 5: 15.1 usec per loop # after
    # -> 1.13x faster (for relative paths)
    commonpath()
    200000 loops, best of 5: 1.62 usec per loop # before
    200000 loops, best of 5: 1.48 usec per loop # after
    # -> 1.09x faster
    

@bedevere-app bedevere-app bot mentioned this pull request Apr 7, 2024
@erlend-aasland
Copy link
Contributor

Note that in general, we do not accept PRs with minor speedups like this. Moreover, any code change risks the introduction of new bugs, as the currently failing CI indicates.

@nineteendo
Copy link
Contributor Author

Also note that this is still a work in progress:

  1. I have not yet done benchmarks on Unix, so I don't know how large the speedups are there.
  2. I'll be investigating the failing tests. They're most likely related to running abspath() of the other platform as the other changes don't rely on any assumptions.

@nineteendo
Copy link
Contributor Author

Also, in the old code this loop calls _get_bothseps() in every iteration without caching the result:

cpython/Lib/ntpath.py

Lines 377 to 378 in e338e1a

while i < n and path[i] not in _get_bothseps(path):
i += 1

For long users (>100 characters) the new code is 1.61x faster, which is not a minor speedup.

@erlend-aasland
Copy link
Contributor

Closing this PR, as the linked issue is closed.

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

Successfully merging this pull request may close these issues.

None yet

2 participants