importlib: Read distribution name/version from metadata directory names, if possible

[ichard26]

importlib does not cache metadata in-memory, so querying even simple attributes like distribution names and versions can quickly become expensive (as each access requires reading METADATA). Fortunately, Distribution.canonical_name is optimized to parse the metadata directory name to query the name if possible. This commit extends this optimization to the finder implementation and version attribute.

.egg-info directory names tend to not include the version so they are not considered for optimizing version lookup.

simplewheel-2.0-1-py2.py3-none-any.whl had to be modified to rename the .dist-info directory which mistakenly included the wheel build tag (in violation of the wheel specification).

simplewheel/__init__.py
simplewheel-2.0-1.dist-info/DESCRIPTION.rst
simplewheel-2.0-1.dist-info/metadata.json
simplewheel-2.0-1.dist-info/top_level.txt
simplewheel-2.0-1.dist-info/WHEEL
simplewheel-2.0-1.dist-info/METADATA
simplewheel-2.0-1.dist-info/RECORD

Otherwise, it was mistaken for part of the version and led pip to think the wheel was a post-release, breaking tests...

This caught my eye when I was profiling the performance of various commands. iter_all_distributions() was often a noticeable chunk of the profile. For example, reading METADATA is responsible for 15% of the total pip check runtime in my Python 3.11.7 environment with 66 packages installed.

cProfile graph

With this patch, the percentage decreases to 5% (only reading Requires-Dist now requires expensive file IO)

cProfile graph

[ichard26]

@uranusjr I know you contributed the importlib metadata backend, so I'd appreciate it if you could you review this! Do you think wheels with invalid metadata directories are frequent enough in the wild to render this untenable? Thanks!

[uranusjr]

The reason we didn’t do this is because some people rely on the “real” name they gave to the package, not the canonical name. This is an artistic choice that they value very high. We can probably use the version (since Version normalises it anyway).

[ichard26]

Hmm, yeah that's a good point. However, I do think the optimisation is still safe here as the finder already normalises the distribution names (it doesn't use raw_name anywhere else)

pip/src/pip/_internal/metadata/importlib/_envs.py

Lines 61 to 71 in 3f3bc60

	for dist in importlib.metadata.distributions(path=[location]):
	info_location = get_info_location(dist)
	try:
	raw_name = get_dist_name(dist)
	except BadMetadata as e:
	logger.warning("Skipping %s due to %s", info_location, e.reason)
	continue
	normalized_name = canonicalize_name(raw_name)
	if normalized_name in self._found_names:
	continue
	self._found_names.add(normalized_name)

If I remove the optimization from get_dist_name and only use it directly in Distribution.canonical_name and _DistributionFinder.find_impl, would that be okay?

I'll take a closer look at the Version attribute and try to find ways it would lead to undesirable behaviour sometime later this week.

[uranusjr]

I believe we use the raw name in some places, such as in pip show and pip list. Or maybe they don’t use get_dist_name? If this function is only used to get the canonical name, we should just rename it and make it always return the canonicalised value directly.

[ichard26]

@uranusjr sorry I should've clarified that get_dist_name is only used in those two places (the finder and Distribution canonical name property). I'll rename it 👍

[ichard26]

I was profiling pip and I stumbled on a clearer profile to sell the benefit of this optimisation. Reinstalling a local wheel without dependencies spends a lot of time (barring imports) within get_dist_name() with 70 packages installed.

python -m pip install --no-deps six-1.16.0-py2.py3-none-any.whl --force-reinstall

There are five calls to get_distribution() (setuptools, six, six, pip, pip) and numerous calls to iter_all_distributions() which each require a full distribution search, calling get_dist_name() on every discovered distribution.

[ichard26]

Thank you for the review @pradyunsg and @uranusjr!