check_case_conflict is slow in a repo with many files/directories on windows #625
Description
Hello!
I recently updated to a more recent version of this repo for my team's git hooks and we noticed that check_case_conflict got a lot slower. I believe I've tracked down what's going on and I have a suggested improvement, though I'm not sure if it's a great approach.
For context, this is the Legends of Runeterra game's monorepo :) It's got around 1m files and some are pretty deeply nested in directories (a little bit more on this at https://technology.riotgames.com/news/legends-runeterra-cicd-pipeline if anyone's curious)
Testing done using Python 3.6.3 on Windows 10 with pre-commit-hooks==4.0.1 and pre-commit==2.13.0
#575 introduced also checking directories, which is great! But this snippet ends up being slow because os.path.dirname is slow (at least on windows :( )
def parents(file: str) -> Iterator[str]: file = os.path.dirname(file) while file: yield file file = os.path.dirname(file) I used cProfile to run this against a single file:
from pre_commit_hooks import check_case_conflict import cProfile cProfile.run('check_case_conflict.find_conflicting_filenames("Jenkinsfile")') results:
λ py test.py 41899740 function calls in 20.114 seconds Ordered by: standard name ncalls tottime percall cumtime percall filename:lineno(function) 1 0.047 0.047 20.114 20.114 <string>:1(<module>) 4 0.000 0.000 0.000 0.000 _weakrefset.py:38(_remove) 4 0.000 0.000 0.000 0.000 _weakrefset.py:81(add) 3 0.000 0.000 0.277 0.092 check_case_conflict.py:13(lower_set) 3 0.186 0.062 0.277 0.092 check_case_conflict.py:14(<setcomp>) 3189815 1.179 0.000 17.790 0.000 check_case_conflict.py:17(parents) 2 0.000 0.000 18.831 9.416 check_case_conflict.py:24(directories_for) 2 1.042 0.521 18.831 9.416 check_case_conflict.py:25(<setcomp>) 1 0.147 0.147 20.066 20.066 check_case_conflict.py:28(find_conflicting_filenames) 3189815 3.710 0.000 5.528 0.000 ntpath.py:121(splitdrive) 3189815 7.833 0.000 15.701 0.000 ntpath.py:199(split) 3189815 0.910 0.000 16.611 0.000 ntpath.py:240(dirname) 3189815 0.813 0.000 1.216 0.000 ntpath.py:33(_get_bothseps) 2 0.000 0.000 0.000 0.000 subprocess.py:1023(_internal_poll) 2 0.000 0.000 0.022 0.011 subprocess.py:1040(wait) 2 0.000 0.000 0.665 0.332 subprocess.py:1067(_communicate) 22 0.000 0.000 0.000 0.000 subprocess.py:179(Close) ... I messed with it a bit locally and this approach seems to perform significantly better:
def parents(file: str) -> Iterator[str]: path_parts = file.split(os.sep) file = path_parts[-1] while path_parts: yield file file = path_parts.pop() λ py test.py 1962384 function calls in 2.469 seconds Ordered by: standard name ncalls tottime percall cumtime percall filename:lineno(function) 1 0.042 0.042 2.469 2.469 <string>:1(<module>) 4 0.000 0.000 0.000 0.000 _weakrefset.py:38(_remove) 4 0.000 0.000 0.000 0.000 _weakrefset.py:81(add) 3 0.000 0.000 0.283 0.094 check_case_conflict.py:13(lower_set) 3 0.195 0.065 0.283 0.094 check_case_conflict.py:14(<setcomp>) 784800 0.369 0.000 0.670 0.000 check_case_conflict.py:17(parents) 2 0.000 0.000 1.079 0.539 check_case_conflict.py:26(directories_for) 2 0.409 0.204 1.079 0.539 check_case_conflict.py:27(<setcomp>) 1 0.231 0.231 2.427 2.427 check_case_conflict.py:30(find_conflicting_filenames) 2 0.000 0.000 0.000 0.000 subprocess.py:1023(_internal_poll) 2 0.000 0.000 0.020 0.010 subprocess.py:1040(wait) 2 0.000 0.000 0.695 0.348 subprocess.py:1067(_communicate) ... But it doesn't really feel good doing this instead of using the built-in os.path stuff. I'm not really sure how correct/robust/multi-OS-compatible this is.
I'm curious if anyone has alternative ideas! But also if this is simply out of scope for this tool that's fine too :) Thanks for reading!