Remove hot uses of pathlib

[bonzini]

pathlib's implementation of Path iteration is expensive; "foo.parents" has quadratic complexity when building it:

        return self._path._from_parsed_parts(self._drv, self._root,
                                             self._tail[:-idx - 1])

and comparisons performed by "path in file.parents" potentially have the same issue. Introduce a new function that checks whether a file is under a path in the same way, removing usage of Path from the biggest hotspot.

[eli-schwartz]

Introduce a new function that checks whether a file is under a path in the same way, removing usage of Path from the biggest hotspot.

Isn't this just os.path.commonpath? This is a well-known problem with pathlib -- it loves to reimplement os.path via complicated and fiddly data structures with questionable correctness characteristics. And apparently questionable performance characteristics too...

[bonzini]

It's similar to commonpath indeed, but 1) it returns a boolean 2) it only operates on two paths 3) it is not symmetrical (one argument is the path and the other is a file) 4) it is faster.

And apparently questionable performance characteristics too...

Yeah, I like pathlib's ergonomics but I draw the line at quadratic implementations...

[eli-schwartz]

It's similar to commonpath indeed, but 1) it returns a boolean 2) it only operates on two paths 3) it is not symmetrical (one argument is the path and the other is a file) 4) it is faster.

Faster, maybe. I haven't timed it. But with regard to returning a boolean etc. the point of commonpath isn't "this function returns whether a path is an ancestor" but rather that it's the trivial building block for such a check.

It's code we already use in several places:

if os.path.commonpath(x, y) == x:
    do_thing_with(y)

[bonzini]

Yes, it is faster: :)

>>> timeit.timeit('path_starts_with("/usr/bin/", "/usr/bin/foo/bar/baz")',  
     globals={'path_starts_with':path_starts_with, 'is_windows': 'windows', 'os': os})
5.237490472005447

>>> timeit.timeit('path_starts_with("/usr/bin/foo/bar/baz", "/usr/bin/")',  
     globals={'path_starts_with':path_starts_with, 'is_windows': 'windows', 'os': os})
6.680338967334726

>>> timeit.timeit('os.path.commonpath(["/usr/bin/", "/usr/bin/foo/bar/baz"]) == "/usr/bin"',
     globals={'path_starts_with':path_starts_with, 'is_windows': 'windows', 'os': os})
13.45890446798876

Also it doesn't crash and burn if you have different drives on Windows. :) Should I adjust other uses of commonpath?

[eli-schwartz]

Also it doesn't crash and burn if you have different drives on Windows. :)

Yup, we catch ValueError for those. So I suppose it may anyways be worth extracting into a dedicated function. :P

Should I adjust other uses of commonpath?

Yes, please. If we need this function for performance in a hot path anyway, then we might as well use it everywhere the same code pattern is used.