Skip to content

Conversation

@alexeypa
Copy link

@alexeypa alexeypa commented Sep 29, 2024

Currently the append, append_const, and extend actions make a copy of the target list every time the action is called. It becomes rather slow with very long option lists.

Using the repro script from the issue description I could see the following results before and after this change.

Before:

➜  cpython git:(69a4063ca5) ✗ ./repro.py                                      
     256 iterations: 0:00:00.002216 s
     512 iterations: 0:00:00.003734 s
    1024 iterations: 0:00:00.009686 s
    2048 iterations: 0:00:00.018806 s
    4096 iterations: 0:00:00.048392 s
    8192 iterations: 0:00:00.130792 s
   16384 iterations: 0:00:00.399441 s
   32768 iterations: 0:00:01.341005 s
   65536 iterations: 0:00:05.148916 s
  131072 iterations: 0:00:20.325517 s

After:

➜  cpython git:(gh-96859_take2) ✗ ./repro.py                
     256 iterations: 0:00:00.001772 s
     512 iterations: 0:00:00.002994 s
    1024 iterations: 0:00:00.008132 s
    2048 iterations: 0:00:00.012443 s
    4096 iterations: 0:00:00.025432 s
    8192 iterations: 0:00:00.052343 s
   16384 iterations: 0:00:00.099193 s
   32768 iterations: 0:00:00.189960 s
   65536 iterations: 0:00:00.388533 s
  131072 iterations: 0:00:00.765152 s

@bedevere-app
Copy link

bedevere-app bot commented Sep 29, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

…tend action

Currently the append, append_const, and extend actions make a copy of the target
list every time the action is called. It becomes rather slow with very long
option lists.
@bedevere-app
Copy link

bedevere-app bot commented Sep 29, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@alexeypa
Copy link
Author

Nevemind, changing the signature of __call__ on 2cc48ba seems like a no go. It looks like the caller can pass a custom action class to action, so this will break existing clients. Let me see if there is a better way to do this.

@alexeypa alexeypa closed this Sep 29, 2024
@alexeypa alexeypa deleted the gh-96859_take2 branch September 29, 2024 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant