poetry-core icon indicating copy to clipboard operation
poetry-core copied to clipboard

Cache `Path.is_dir` calls

Open ethantkoenig opened this issue 1 year ago • 2 comments

Profiles of a poetry lock --no-update command (run on a closed-source repo I am unable to share) are showing that this Path.is_dir call is a hotspot (~0.8s out of a ~4.8s), and calls are often made with the same path multiple times (~1260 calls with ~440 unique paths):

https://github.com/python-poetry/poetry-core/blob/1bdbd900c4fc1e2d4ed1ef15dfbcafdcb66dd892/src/poetry/core/packages/dependency.py#L364

Caching Path.is_dir trims roughly ~0.5s off of the time spent in Path.is_dir calls.

(I'm not familiar with this code, but the fact that is_python_project is already cached makes me assume that caching Path.is_dir calls is also safe.)

ethantkoenig avatar Oct 14 '24 05:10 ethantkoenig

@ethantkoenig Out of interest: Is only Path.is_dir a hotspot or is Dependency.create_from_pep_508 still an hotspot after caching Path.is_dir? It is far more risky, but I'd like to know if thinking about caching Dependency.create_from_pep_508 makes sense. We had to adapt the caching so that it returns a copy (Dependency.clone) of the dependency similar as in https://stackoverflow.com/questions/54909357/how-to-get-functools-lru-cache-to-return-new-instances so it might not make sense in the end.

radoering avatar Oct 15 '24 16:10 radoering

@ethantkoenig Out of interest: Is only Path.is_dir a hotspot or is Dependency.create_from_pep_508 still an hotspot after caching Path.is_dir? It is far more risky, but I'd like to know if thinking about caching Dependency.create_from_pep_508 makes sense. We had to adapt the caching so that it returns a copy (Dependency.clone) of the dependency similar as in https://stackoverflow.com/questions/54909357/how-to-get-functools-lru-cache-to-return-new-instances so it might not make sense in the end.

With this change, create_from_pep_508 is taking ~0.4s, with ~0.2s spent in cached_is_dir/is_dir

ethantkoenig avatar Oct 17 '24 05:10 ethantkoenig