GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-109653: Reduce import overhead of uuid module on Linux by hauntsaninja · Pull Request #115160 · python/cpython

Conversation

@hauntsaninja

@hauntsaninja hauntsaninja commented Feb 8, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This follows in the footsteps of #21586
This speeds up import uuid by about 6ms on Linux.

Before:

λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      20.4 ms ±   0.4 ms    [User: 16.7 ms, System: 3.8 ms]
  Range (min … max):    19.6 ms …  21.8 ms    136 runs

After:

λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      14.5 ms ±   0.3 ms    [User: 11.5 ms, System: 3.2 ms]
  Range (min … max):    13.9 ms …  16.0 ms    175 runs
  • Issue: Improve import time of various stdlib modules #109653

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@hauntsaninja
Reduce import overhead of uuid module
ed29bd6 
This follows in the footsteps of python#21586
This speeds up import uuid by about 6ms on Linux.

Before:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      20.4 ms ±   0.4 ms    [User: 16.7 ms, System: 3.8 ms]
  Range (min … max):    19.6 ms …  21.8 ms    136 runs
```
After:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      14.5 ms ±   0.3 ms    [User: 11.5 ms, System: 3.2 ms]
  Range (min … max):    13.9 ms …  16.0 ms    175 runs
```
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 8, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 8, 2024
Improve import time of various stdlib modules #109653
Closed
@hauntsaninja
hauntsaninja requested a review from zooba February 8, 2024 08:50
@blurb-it
📜🤖 Added by blurb_it.
4dda652
@hauntsaninja hauntsaninja changed the title gh-109653: Reduce import overhead of uuid module gh-109653: Reduce import overhead of uuid module on Linux Feb 8, 2024
@zooba
zooba removed their request for review February 8, 2024 14:14
@zooba

zooba commented Feb 8, 2024

Copy link
Copy Markdown
Member

I'm not the right person to review this.

You need someone who knows whether sys.platform == "linux" implies the same things as the platform module. In general, they don't, so if you want my review I would reject this change as being too risky.
hauntsaninja reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja

hauntsaninja commented Feb 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Thanks for the quick response! I'll wait for someone else to review.

The value of sys.platform comes from

cpython/configure

Line 4296 in 1032326

case $MACHDEP in
while the value of platform.system comes from os.uname + munging and falling back to sys.platform

Note we don't need a one-to-one correspondence here (and I'm not sure there is one, e.g. on ancient Python versions it was possible to have sys.platform == "linux2" while presumably platform.system() was still Linux). But we just need implication in one direction since we preserve the else branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ofek

ofek commented Feb 28, 2024

Copy link
Copy Markdown
Contributor

This seems correct to me; I don't see any risk to this change.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hauntsaninja
hauntsaninja merged commit 68b8fff into python:main Mar 6, 2024
@hauntsaninja
hauntsaninja deleted the uuid-import branch March 6, 2024 22:39
@bedevere-app bedevere-app Bot removed the awaiting core review label Mar 6, 2024
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@hauntsaninja @adorilson
pythongh-109653: Reduce import overhead of uuid module on Linux (pyth…
d4fa77a 
…on#115160)

This follows in the footsteps of python#21586
This speeds up import uuid by about 6ms on Linux.

Before:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      20.4 ms ±   0.4 ms    [User: 16.7 ms, System: 3.8 ms]
  Range (min … max):    19.6 ms …  21.8 ms    136 runs
```
After:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      14.5 ms ±   0.3 ms    [User: 11.5 ms, System: 3.2 ms]
  Range (min … max):    13.9 ms …  16.0 ms    175 runs
```
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@hauntsaninja @diegorusso
pythongh-109653: Reduce import overhead of uuid module on Linux (pyth…
965c421 
…on#115160)

This follows in the footsteps of python#21586
This speeds up import uuid by about 6ms on Linux.

Before:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      20.4 ms ±   0.4 ms    [User: 16.7 ms, System: 3.8 ms]
  Range (min … max):    19.6 ms …  21.8 ms    136 runs
```
After:
```
λ hyperfine -w 4 "./python -c 'import uuid'"
Benchmark 1: ./python -c 'import uuid'
  Time (mean ± σ):      14.5 ms ±   0.3 ms    [User: 11.5 ms, System: 3.2 ms]
  Range (min … max):    13.9 ms …  16.0 ms    175 runs
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@hauntsaninja @zooba @ofek
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.