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-100562: improve performance ofpathlib.Path.absolute()#100563

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

Conversation

barneygale
Copy link
Contributor

@barneygalebarneygale commentedDec 28, 2022•
edited by miss-islington
Loading

Increase performance of theabsolute() method by callingos.getcwd() directly, rather than using thePath.cwd() class method. This avoids constructing an extraPath object (and the parsing/normalization that comes with it).

Decrease performance of thecwd() class method by calling thePath.absolute() method, rather than usingos.getcwd() directly. This involves constructing an extraPath object. We do this to maintain a longstanding pattern whereos functions are called from only one place, which allows them to be more readily replaced by users. Ascwd() 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

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 avoidsconstructing an extra `Path` object (and the parsing/normalization thatcomes with it).Decrease performance of the `cwd()` class method by calling the`Path.absolute()` method, rather than using `os.getcwd()` directly. Thisinvolves constructing an extra `Path` object. We do this to maintain alongstanding pattern where `os` functions are called from only one place,which allows them to be more readily replaced by users. As `cwd()` isgenerally called at most once within user programs, it's a good bargain.
@AlexWaygoodAlexWaygood added performancePerformance or resource usage 3.12only security fixes labelsDec 28, 2022
@AlexWaygoodAlexWaygood self-requested a reviewDecember 28, 2022 01:02
Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

This looks great to me. I agree that it's much more important to optimisePath.absolute() thanPath.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?

@barneygale
Copy link
ContributorAuthor

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

AlexWaygood reacted with hooray emoji

…ic0Z0.rstCo-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@AlexWaygood
Copy link
Member

AlexWaygood commentedJan 5, 2023•
edited
Loading

I think all the tests are failing because Guido just broke themain 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!

@AlexWaygoodAlexWaygood linked an issueJan 5, 2023 that may beclosed by this pull request
@miss-islington
Copy link
Contributor

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

@miss-islingtonmiss-islington merged commit7fba99e intopython:mainJan 5, 2023
@bedevere-bot
Copy link

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

Hi! The buildbotARM Raspbian 3.x has failed when building commit7fba99e.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (7fba99e) or if it is a false positive.
  5. 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 0From https://github.com/python/cpython * branch                  main       -> FETCH_HEADNote:switching to '7fba99eadb3349a6d49d02f13b1fddf44c674393'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo 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 falseHEAD 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
Copy link
Member

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

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

barneygale reacted with thumbs up emoji

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

@merwokmerwokmerwok left review comments

@eryksuneryksuneryksun left review comments

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

@brettcannonbrettcannonAwaiting requested review from brettcannon

Assignees
No one assigned
Labels
3.12only security fixesperformancePerformance or resource usagetopic-pathlib
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve performance of pathlib.Path.absolute()
6 participants
@barneygale@AlexWaygood@miss-islington@bedevere-bot@merwok@eryksun
[8]ページ先頭
©2009-2025 Movatter.jp