gh-100562: improve performance of `pathlib.Path.absolute()` #100563

barneygale · 2022-12-28T00:43:34Z

Increase performance of the absolute() method by calling os.getcwd() directly, rather than using the Path.cwd() class method. This avoids constructing an extra Path object (and the parsing/normalization that comes with it).

Decrease performance of the cwd() class method by calling the Path.absolute() method, rather than using os.getcwd() directly. This involves constructing an extra Path object. We do this to maintain a longstanding pattern where os functions are called from only one place, which allows them to be more readily replaced by users. As cwd() is generally called at most once within user programs, it's a good bargain.

# before
$ ./python -m timeit -s 'from pathlib import Path; p = Path("foo", "bar")' 'p.absolute()'
50000 loops, best of 5: 9.04 usec per loop
# after
$ ./python -m timeit -s 'from pathlib import Path; p = Path("foo", "bar")' 'p.absolute()'
50000 loops, best of 5: 5.02 usec per loop

Issue: Improve performance of pathlib.Path.absolute() #100562

Automerge-Triggered-By: GH:AlexWaygood

Increase performance of the `absolute()` method by calling `os.getcwd()` directly, rather than using the `Path.cwd()` class method. This avoids constructing an extra `Path` object (and the parsing/normalization that comes with it). Decrease performance of the `cwd()` class method by calling the `Path.absolute()` method, rather than using `os.getcwd()` directly. This involves constructing an extra `Path` object. We do this to maintain a longstanding pattern where `os` functions are called from only one place, which allows them to be more readily replaced by users. As `cwd()` is generally called at most once within user programs, it's a good bargain.

Lib/pathlib.py

AlexWaygood

This looks great to me. I agree that it's much more important to optimise Path.absolute() than Path.cwd(), and I can reproduce the huge speedup.

Is the plan still to hold off merging this for now until #100563 (comment) is resolved, or can this be merged at any time?

Misc/NEWS.d/next/Library/2022-12-28-00-28-43.gh-issue-100562.Hic0Z0.rst

barneygale · 2023-01-05T21:21:58Z

Thanks for the review!

Is the plan still to hold off merging this for now until #100563 (comment) is resolved, or can this be merged at any time?

I think this can be merged any time. The backport for the fix for Eryk's issue will be slightly more complicated, but I will cope :D

…ic0Z0.rst Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood · 2023-01-05T21:25:38Z

I think all the tests are failing because Guido just broke the main branch accidentally :-)

But I'll re-run the tests here as soon as the fix in #100778 is merged (just to be on the safe side), then I'll merge!

miss-islington · 2023-01-05T22:11:49Z

Status check is done, and it's a success ✅.

bedevere-bot · 2023-01-05T23:19:30Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM Raspbian 3.x has failed when building commit 7fba99e.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/424/builds/3233) and take a look at the build logs.
Check if the failure is related to this commit (7fba99e) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/424/builds/3233

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

411 tests OK.

10 slowest tests:

test_venv: 7 min 32 sec
test_largefile: 6 min 29 sec
test_tokenize: 6 min 23 sec
test_multiprocessing_spawn: 4 min 28 sec
test_zipfile: 4 min 28 sec
test_multiprocessing_main_handling: 3 min 21 sec
test_concurrent_futures: 3 min 14 sec
test_asyncio: 3 min 1 sec
test_multiprocessing_forkserver: 1 min 48 sec
test_gdb: 1 min 42 sec

1 test altered the execution environment:
test_concurrent_futures

21 tests skipped:
test_check_c_globals test_devpoll test_idle test_ioctl test_kqueue
test_launcher test_msilib test_peg_generator test_perf_profiler
test_startfile test_tcl test_tix test_tkinter test_ttk
test_ttk_textonly test_turtle test_winconsoleio test_winreg
test_winsound test_wmi test_zipfile64

Total duration: 31 min 19 sec

Click to see traceback logs

remote: Enumerating objects: 22, done.        
remote: Counting objects:   4% (1/22)        
remote: Counting objects:   9% (2/22)        
remote: Counting objects:  13% (3/22)        
remote: Counting objects:  18% (4/22)        
remote: Counting objects:  22% (5/22)        
remote: Counting objects:  27% (6/22)        
remote: Counting objects:  31% (7/22)        
remote: Counting objects:  36% (8/22)        
remote: Counting objects:  40% (9/22)        
remote: Counting objects:  45% (10/22)        
remote: Counting objects:  50% (11/22)        
remote: Counting objects:  54% (12/22)        
remote: Counting objects:  59% (13/22)        
remote: Counting objects:  63% (14/22)        
remote: Counting objects:  68% (15/22)        
remote: Counting objects:  72% (16/22)        
remote: Counting objects:  77% (17/22)        
remote: Counting objects:  81% (18/22)        
remote: Counting objects:  86% (19/22)        
remote: Counting objects:  90% (20/22)        
remote: Counting objects:  95% (21/22)        
remote: Counting objects: 100% (22/22)        
remote: Counting objects: 100% (22/22), done.        
remote: Compressing objects:  12% (1/8)        
remote: Compressing objects:  25% (2/8)        
remote: Compressing objects:  37% (3/8)        
remote: Compressing objects:  50% (4/8)        
remote: Compressing objects:  62% (5/8)        
remote: Compressing objects:  75% (6/8)        
remote: Compressing objects:  87% (7/8)        
remote: Compressing objects: 100% (8/8)        
remote: Compressing objects: 100% (8/8), done.        
remote: Total 13 (delta 10), reused 6 (delta 5), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '7fba99eadb3349a6d49d02f13b1fddf44c674393'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 7fba99eadb gh-100562: improve performance of `pathlib.Path.absolute()` (GH-100563)
Switched to and reset branch 'main'

Objects/obmalloc.c:776:1: warning: ‘always_inline’ function might not be inlinable [-Wattributes]
  776 | arena_map_get(pymem_block *p, int create)
      | ^~~~~~~~~~~~~

make: *** [Makefile:1906: buildbottest] Error 3

AlexWaygood · 2023-01-05T23:22:13Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Looks like a fluke to me, I can't see how this change would mean that test_concurrent_futures would start leaking changes into the test environment

barneygale requested a review from brettcannon as a code owner December 28, 2022 00:43

bedevere-bot mentioned this pull request Dec 28, 2022

Improve performance of pathlib.Path.absolute() #100562

Closed

bedevere-bot added the awaiting review label Dec 28, 2022

AlexWaygood added performance Performance or resource usage 3.12 bugs and security fixes labels Dec 28, 2022

AlexWaygood self-requested a review December 28, 2022 01:02

eryksun reviewed Dec 28, 2022

View reviewed changes

Lib/pathlib.py Show resolved Hide resolved

merwok reviewed Dec 29, 2022

View reviewed changes

Lib/pathlib.py Show resolved Hide resolved

Improve cwd() docstring, add comment.

c71e364

AlexWaygood approved these changes Jan 5, 2023

View reviewed changes

Misc/NEWS.d/next/Library/2022-12-28-00-28-43.gh-issue-100562.Hic0Z0.rst Outdated Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting review labels Jan 5, 2023

AlexWaygood added the topic-pathlib label Jan 5, 2023

Update Misc/NEWS.d/next/Library/2022-12-28-00-28-43.gh-issue-100562.H…

43e5d21

…ic0Z0.rst Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Merge branch 'main' into pythongh-100562-path-absolute-perf

031c322

AlexWaygood added the 🤖 automerge label Jan 5, 2023

AlexWaygood linked an issue Jan 5, 2023 that may be closed by this pull request

Improve performance of pathlib.Path.absolute() #100562

Closed

miss-islington merged commit 7fba99e into python:main Jan 5, 2023

bedevere-bot removed the awaiting merge label Jan 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-100562: improve performance of `pathlib.Path.absolute()` #100563

gh-100562: improve performance of `pathlib.Path.absolute()` #100563

barneygale commented Dec 28, 2022 •

edited by miss-islington

Loading

AlexWaygood left a comment

barneygale commented Jan 5, 2023

AlexWaygood commented Jan 5, 2023 •

edited

Loading

miss-islington commented Jan 5, 2023

bedevere-bot commented Jan 5, 2023

AlexWaygood commented Jan 5, 2023

gh-100562: improve performance of pathlib.Path.absolute() #100563

gh-100562: improve performance of pathlib.Path.absolute() #100563

Conversation

barneygale commented Dec 28, 2022 • edited by miss-islington Loading

AlexWaygood left a comment

Choose a reason for hiding this comment

barneygale commented Jan 5, 2023

AlexWaygood commented Jan 5, 2023 • edited Loading

miss-islington commented Jan 5, 2023

bedevere-bot commented Jan 5, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

AlexWaygood commented Jan 5, 2023

gh-100562: improve performance of `pathlib.Path.absolute()` #100563

gh-100562: improve performance of `pathlib.Path.absolute()` #100563

barneygale commented Dec 28, 2022 •

edited by miss-islington

Loading

AlexWaygood commented Jan 5, 2023 •

edited

Loading