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. This commit updates parse_known_args() to only copy on the first action call.

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

Before:

➜  cpython git:(c00964ecd5) ✗ ./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_take3) ✗ ./repro.py                   
     256 iterations: 0:00:00.002016 s
     512 iterations: 0:00:00.003760 s
    1024 iterations: 0:00:00.006729 s
    2048 iterations: 0:00:00.013103 s
    4096 iterations: 0:00:00.026316 s
    8192 iterations: 0:00:00.053061 s
   16384 iterations: 0:00:00.099570 s
   32768 iterations: 0:00:00.191617 s
   65536 iterations: 0:00:00.403077 s
  131072 iterations: 0:00:00.781361 s

…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. This commit updates parse_known_args() to only copy on the first
action call.
@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.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Oct 2, 2024
…nd 'extend'

Previously, the target list was copied every time the action was taken.
Now only the default value is copied the first time the action is taken.
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Oct 2, 2024
…nd 'extend'

Previously, the target list was copied every time the action was taken.
Now only the default value is copied the first time the action is taken.
@serhiy-storchaka
Copy link
Member

Thank you @alexeypa, but I do not like moving action-specific code to general code. I think that simple approach (#124909) would work as well.

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.

2 participants