-
-
Notifications
You must be signed in to change notification settings - Fork 117
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
Rect.clipline() optimization #2563
Rect.clipline() optimization #2563
Conversation
Did that to get rid of that XDECREF. Decref takes time and XDRECREF takes even more, so since the not normalized rect case doesn't happen often i thought to move everything related to rect_copy int there to go as fast as possible in the general case. Nothing major so we could revert if strictly necessary. |
Ok, I was wondering what your angle was. Isn't the difference between XDECREF and DECREF just a NULL check? It doesn't seem worth it to me to make the code longer and harder to read for a single NULL check. My preference is to take the change out of this PR, because the rest of it looks straightforward to me. My bet is that the vast majority of performance here is coming from FASTCALL. Then in the future if you want to re-add this DECREF change it can be tested and weighed on its own. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the PR 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, faster with less code!
This should improve Rect.clipline() performance a bit. Now using fastcall. I get between a 20-30% performance improvement while respecting compat. This could improve further once the
Line
object is in the geometry module.Program:
Results: