-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Scalar field improvements #103
Conversation
Opens opportunity to e.g. compute lazily.
Use local variable for performance (compiler can't optimize store due to exception possibility). Reduces execution time approximately 30%. Also properly clears min and max if the field is not empty, but all points are invalid.
@dgirardeau Hello, would like to make sure this gets reviewed. |
Ah, I forgot about this PR sorry. Is it only about syntax? |
No, there is a little bit of a behavior change. Please see the commit messages. |
Sorry I fail to see what's optimized / improved 😅 |
|
On which OS have you performed your benchmarks? Not sure to grasp why updating the members would be slower 🤔 |
And just to be sure again, was it in Release or Debug mode? |
I checked on x86_64 Linux in Release mode. The compiler has to keep reading and writing the member variables in memory each loop iteration so that they have the latest value, I'm assuming in case of an exception. Usually this is not such a big deal but for how short the loop is it turns into a measurable overhead. If locals are used then they can be kept in registers. I checked the assembly code. |
Wow, that's good to know. And I guess it must be the same on Windows if that's the explanation ... I guess there are other places where this happens then. |
I did also remove the case through the function where the member variables never get set, maybe having that is why the compiler decided to access memory each iteration. I'm not exactly sure to be honest. |
Please see commits for details.