gh-91247: Performance improvement in list repeating#92286
gh-91247: Performance improvement in list repeating#92286thatbirdguythatuknownot wants to merge 7 commits intopython:mainfrom
Conversation
MojoVampire
left a comment
There was a problem hiding this comment.
You're introducing undefined behavior, and making many changes that aren't related to the actual performance goals of the patch. The actual part that relates to performance (at the very end of the patch) seems okay, but most of the rest of it should be rolled back, as it's unnecessary, unhelpful, or actively harmful.
There was a problem hiding this comment.
Please stop pushing completely unverified code. Make it work, then submit the PR. A handful of commits in a given PR are not unreasonable, but seven commits in the space of minutes, most of them not even high enough quality to compile, let alone work correctly, does not a good PR request make. Make something that works and you're mostly confident of, minimize the number of changes not required to implement the fix (no pointless/harmful shoving assignments in the middle of larger expressions), and squash the commits down to just one or two for the bulk of the actual work.
I know that, but I usually can't verify it because I make changes on the desktop website (I don't know how to use |
You're going to have to learn. It's literally the first thing you do to begin contributing. See the Python Developer's Guide: Setup. You can make all the changes you need to locally, build and verify them, all without pushing a single thing to GitHub or opening a PR. |
I have a local working modified copy of the repo (not by |
|
@thatbirdguythatuknownot @MojoVampire This PR has the same goal and approach as #91247. That PR and the corresponding issue has a detailed analysis of the performance impact (which is quite subtle) |
|
The actual pr is #91482 |
|
This is out of date. Closing. Thank you @thatbirdguythatuknownot . |
issue: #91247