Skip to content

perf: trivial changes#952

Open
radoering wants to merge 1 commit into
python-poetry:mainfrom
radoering:perf/minor-but-trivial
Open

perf: trivial changes#952
radoering wants to merge 1 commit into
python-poetry:mainfrom
radoering:perf/minor-but-trivial

Conversation

@radoering

Copy link
Copy Markdown
Member

Not much gain, but trivial changes that do not affect maintainability.

  1. We already cache parse_constraint (some lines above) so that it just makes sense to also cache parse_marker_version_constraint.
  2. We can avoid the creation of a throwaway empty list.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Using functools.cache makes the cache unbounded; consider functools.lru_cache with a reasonable maxsize (as is likely done for parse_constraint) to avoid unbounded memory growth in long‑running processes.
  • If the project still supports Python versions prior to 3.9, double-check that functools.cache is available in the minimum supported version or align with whatever decorator is used to cache parse_constraint for consistency.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Using `functools.cache` makes the cache unbounded; consider `functools.lru_cache` with a reasonable `maxsize` (as is likely done for `parse_constraint`) to avoid unbounded memory growth in long‑running processes.
- If the project still supports Python versions prior to 3.9, double-check that `functools.cache` is available in the minimum supported version or align with whatever decorator is used to cache `parse_constraint` for consistency.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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.

1 participant