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

[nineteendo]

Benchmark

script

# test.sh
echo 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:

def test1(paths):
    sep = '/'
    try:
        isabs, = {p.startswith(sep) for p in paths}
    except ValueError:
        raise ValueError("Can't mix absolute and relative paths") from None

    prefix = sep if isabs else sep[:0]
    return prefix

def test2(paths):
    sep = '/'
    if len({p.startswith(sep) for p in paths}) != 1:
        raise ValueError("Can't mix absolute and relative paths")

    prefix = sep if paths[0].startswith(sep) else sep[:0]
    return prefix

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.