gh-117641: Use set comprehension for posixpath.commonpath()

[nineteendo]

Benchmark

script

# test.shecho 1 item && python -m timeit -s "import before.posixpath""before.posixpath.commonpath(['foo'])"&& python -m timeit -s "import after.posixpath""after.posixpath.commonpath(['foo'])"echo 10 items && python -m timeit -s "import before.posixpath""before.posixpath.commonpath(['foo'] * 10)"&& python -m timeit -s "import after.posixpath""after.posixpath.commonpath(['foo'] * 10)"echo 100 items && python -m timeit -s "import before.posixpath""before.posixpath.commonpath(['foo'] * 100)"&& python -m timeit -s "import after.posixpath""after.posixpath.commonpath(['foo'] * 100)"

1 item
200000 loops, best of 5: 1.81 usec per loop # before
200000 loops, best of 5: 1.47 usec per loop # after
# -> 1.23x faster
10 items
50000 loops, best of 5: 5.25 usec per loop # before
50000 loops, best of 5: 4.66 usec per loop # after
# -> 1.13x faster
100 items
5000 loops, best of 5: 39.9 usec per loop # before
10000 loops, best of 5: 37.3 usec per loop # after
# -> 1.07x faster

• Issue: Use set comprehension for posixpath.commonpath() #117641

[nineteendo]

@serhiy-storchaka, do you have suggestions here?

[serhiy-storchaka]

I understand the use of set comprehension instead of generator, it can add a tiny speed up. But why get rid of unpack?

[nineteendo]

I wanted to make the code more readable. But I'll revert it as it's slower:

deftest1(paths):
sep='/'try:
isabs, = {p.startswith(sep) forpinpaths}
exceptValueError:
raiseValueError("Can't mix absolute and relative paths") fromNoneprefix=sepifisabselsesep[:0]
returnprefixdeftest2(paths):
sep='/'iflen({p.startswith(sep) forpinpaths}) !=1:
raiseValueError("Can't mix absolute and relative paths")
prefix=sepifpaths[0].startswith(sep) elsesep[:0]
returnprefix

wannes@Stefans-iMac Desktop % python -m timeit -s "import test""test.test1(['foo'])"&& python -m timeit -s "import test""test.test2(['foo'])"
1000000 loops, best of 5: 369 nsec per loop # unpack
500000 loops, best of 5: 451 nsec per loop # no unpack# -> 1.22x slower

[nineteendo]

@erlend-aasland, do you think this can be merged now? I addressed serhiy-storchaka's question.

[erlend-aasland]

@erlend-aasland, do you think this can be merged now? I addressed serhiy-storchaka's question.

I'll run PGO benchmarks for the updated PR first.

UPDATE: I'm getting between 5% and 20% speedups, similar to the speedup in the PR description.

[nineteendo]

@erlend-aasland, could you merge it? It has been approved by serhiy.

[erlend-aasland]

@nineteendo: yes, it was on my list; there is no need for the extra ping. There is no hurry; I'll land it tomorrow morning. Please avoid unneeded pings.

[nineteendo]

Thanks.

Please avoid unneeded pings.

Sorry, sometimes I get no response, and then I don't know if my message was even read.