Skip to content

Cache the _location_without_md5.#2098

Closed
jaraco wants to merge 2 commits into
masterfrom
feature/faster-distribution-hashcmp
Closed

Cache the _location_without_md5.#2098
jaraco wants to merge 2 commits into
masterfrom
feature/faster-distribution-hashcmp

Conversation

@jaraco

@jaraco jaraco commented May 10, 2020

Copy link
Copy Markdown
Member

Fixes #2089. Alternate to #2090.

@gotcha

gotcha commented May 11, 2020

Copy link
Copy Markdown

This relies on the fact that location should be set only once. Is there anything that avoids to set it twice ?
This is why #2090 makes location a property.

@jaraco

jaraco commented May 12, 2020

Copy link
Copy Markdown
Member Author

There's nothing that prevents it from being set again, but there's also no reason to think that it will be set again.

@gotcha

gotcha commented May 12, 2020

Copy link
Copy Markdown

@jaraco While reading the code to answer to your question on #2089, I notice the following:

  • hashcmp is mainly slow because of _remove_md5_fragment but it uses a lot of properties
  • key and parsed_version are also cached, with two very simple but differing implementations, key uses try except AttributeError; parsed_version uses hasattr

This leads me to the following questions:

  • Should the full hashcmp be cached ? I tend to answer yes because it already relies on other cached properties.
  • Could it be cached simply, like key or parsed_version ? I also tend to answer positively.
  • Which implementation should we choose ? hasattr or try except ? I am not deep enough in Python internals to know which is quicker.
  • Is there a reason to use a different implementation of caching for keyand parsed_version ? Should we unify on the quickest ?
  • What is the reason to use 3.8 cached_property instead of a simpler caching implementation ?

@jaraco

jaraco commented May 13, 2020

Copy link
Copy Markdown
Member Author

I prefer #2108. I'll answer the questions in #2089.

@jaraco jaraco closed this May 13, 2020
@jaraco jaraco deleted the feature/faster-distribution-hashcmp branch May 13, 2020 01:20
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.

pkg_resources.Distribution.hashcmp is slow

2 participants