-
-
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
Made Surface.set_at()
fastcall
#2330
Made Surface.set_at()
fastcall
#2330
Conversation
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.
set_at
is typically called in tight loops, so I think this optimisation is pretty good.
The CI is failing on a warning, which I'm sure you figured by now, just needs an explicit cast.
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 now, thanks! 👍
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.
This conflicts with Scriptline's PR #2111.
Lets try to get that in first, then you can rebase this against that?
Edit: resolved, earlier PR merged.
I'll convert it draft for now, feel free to open it back when the mentioned pr is merged |
To the person merging this: squash and merge this PR so that it doesn't conflict with my other PR. It would probably add me as a co-author of this PR, which you can remove if you want |
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.
Looks good to me, thanks! Nice speed ups!
This PR aims to get a quick and easy perf improvement with fastcall as well as removing a redundant check.
Results:
TLDR, about 34-40% improvement on my machine.
Code Used: