Skip to content

bpo-38436: Improved performance for list addition. #16705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Oct 10, 2019

This PR adds a fast path for BINARY_ADD instructions involving two lists, where the left list has a refcount of exactly 1.

In this case, we instead do a PySequence_InPlaceConcat operation. This has the affect of avoiding quadratic complexity for list summations, by keeping only one intermediate result and extending it in-place.

For (potentially large) lists:

a + b + c + d ...

Currently executed as:

tmp = a + b
tmp = tmp + c
tmp = tmp + d
...

With this change:

tmp = a + b
tmp += c
tmp += d
...

Here are the pyperformance results (optimizations enabled):

EDIT: there are better measurements in the BPO discussion.

This is related to my earlier work in bpo-36229, however this is a much less invasive change that's limited to ceval.c, where our knowledge of context is much better.

https://bugs.python.org/issue38436

@pablogsal
Copy link
Member

Where the benchmarks executed with CPU isolation? I am a bit suspicious of the

- pickle_list: 3.63 us +- 0.08 us -> 3.78 us +- 0.10 us: 1.04x slower (+4%)

If not, can you repeat them with CPU isolation and affinity? Check this for more info

@brandtbucher
Copy link
Member Author

@pablogsal No isolation (I'm on a MacBook) but I can rerun with affinity on two cores.

@pablogsal
Copy link
Member

pablogsal commented Oct 10, 2019

@pablogsal No isolation (I'm on a MacBook) but I can rerun with affinity on two cores.

I can try to run the benchmarks on my machine or the speed.python.org server with isolation, as this is a very sensitive operation.

@brandtbucher
Copy link
Member Author

Thanks @pablogsal - that would probably be best!

@brandtbucher brandtbucher deleted the list-add branch July 21, 2022 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants