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

performance: cache parsed markers, constraints and versions #556

Merged
merged 3 commits into from
Feb 20, 2023

Conversation

radoering
Copy link
Member

@radoering radoering commented Feb 19, 2023

Some more fallout from #530. The parsing of marker strings became a bit slower with measurable effects in real-world examples. It seems in real-world examples, the same marker is used many times for different dependencies. Since markers are immutable, we can just cache the parsed markers and mitigate the performance regression.

Performance for shootout example with warm cache:

command 1.3.2 master PR (only markers) PR (constraints, versions)
lock 46 s 57 s 47 s 35 s
lock --no-update 14 s 18 s 14 s 10 s

(There seems to be no significant increase in peak memory usage due to the caching.)

Update: added times for caching parsed constraints and versions in addition to markers.

dimbleby
dimbleby previously approved these changes Feb 19, 2023
@dimbleby
Copy link
Contributor

looks fine to me though fwiw I don't reproduce any performance delta on the "big" install that I have lying around.

@dimbleby
Copy link
Contributor

if this is real: I wonder whether constraint parsing and version parsing would similarly benefit

@radoering
Copy link
Member Author

if this is real: I wonder whether constraint parsing and version parsing would similarly benefit

Great idea. Although there is more variance in constraints and versions than in markers, it makes a significant difference, too.

I updated the times in the PR description. Further, I tested the change on another largish (non-public) project:

command without PR only markers markers,constraints,versions
poetry lock --no-update 150 s 135 s 115 s

Cache info for private project:

CacheInfo(hits=66985, misses=78, maxsize=None, currsize=78) poetry.core.version.markers.parse_marker
CacheInfo(hits=452083, misses=739, maxsize=None, currsize=739) poetry.core.constraints.version.parser.parse_constraint
CacheInfo(hits=73960, misses=196, maxsize=None, currsize=196) poetry.core.constraints.generic.parser.parse_constraint
CacheInfo(hits=19933, misses=581, maxsize=None, currsize=580) poetry.core.version.pep440.parser.PEP440Parser.parse

The peak memory usage even decreased when caching constraints and versions.

@radoering radoering force-pushed the performance-parse-marker branch from 42b5daa to 4639236 Compare February 19, 2023 14:34
@radoering radoering changed the title performance: cache parsed markers performance: cache parsed markers, constraints and versions Feb 19, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@radoering radoering requested a review from a team February 20, 2023 04:49
@radoering radoering merged commit ea18813 into python-poetry:main Feb 20, 2023
@radoering radoering deleted the performance-parse-marker branch November 24, 2024 12:41
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.

4 participants