Hi all, A code snippet from trunk: void srgb_to_linear( double* c ) { if( *c < 0.04045 ) { *c /= 12.92; } else { *c = pow( (*c+0.055)/1.055, 2.4 ); } }
Alright, this method should not do a NULL check for performance reasons (I assume). But, shouldn't we use a reference in this case then? To signal clearly that the method does not accept NULL pointers without crashing:
void srgb_to_linear( double &c ) { if( c < 0.04045 ) { c /= 12.92; } else { c = pow( (c+0.055)/1.055, 2.4 ); } }
I'm trying to reduce the number of crashes due to simple NULL pointer crashes. I find such bugs satisfying to fix because it takes really only 1 minute to locate the bug and 1 minute to fix and commit it, but........
Ciao, Johan
On Jan 30, 2013, at 9:19 AM, Johan Engelen wrote:
Hi all, A code snippet from trunk: void srgb_to_linear( double* c ) { if( *c < 0.04045 ) { *c /= 12.92; } else { *c = pow( (*c+0.055)/1.055, 2.4 ); } }
Alright, this method should not do a NULL check for performance reasons (I assume). But, shouldn't we use a reference in this case then? To signal clearly that the method does not accept NULL pointers without crashing:
void srgb_to_linear( double &c ) { if( c < 0.04045 ) { c /= 12.92; } else { c = pow( (c+0.055)/1.055, 2.4 ); } }
I'm trying to reduce the number of crashes due to simple NULL pointer crashes. I find such bugs satisfying to fix because it takes really only 1 minute to locate the bug and 1 minute to fix and commit it, but........
Yes, that does seem reasonable.
However... I would wonder about performance. A more normal approach would not be to use a combination in/out variable, but instead take in double and return double. No pointers or references required.
Doing otherwise might be a premature optimization and sometimes is actually counter-productive. It is quite possible that writing code more focused on clarity might allow the compiler to optimize it better. Hard to know without real-world testing with the whole program end-to-end.
Overall I think we have these principals:
* prefer clear code to pre-optimized code. Apply optimizations when they have been measured to be needed. * prefer references to pointers * prefer const references when feasible
On 29-01-13 23:47, Jon Cruz wrote:
On Jan 30, 2013, at 9:19 AM, Johan Engelen wrote: ...
Alright, this method should not do a NULL check for performance reasons (I assume). But, shouldn't we use a reference in this case then? To signal clearly that the method does not accept NULL pointers without crashing: ...
I'm trying to reduce the number of crashes due to simple NULL pointer crashes. I find such bugs satisfying to fix because it takes really only 1 minute to locate the bug and 1 minute to fix and commit it, but........
Yes, that does seem reasonable.
However... I would wonder about performance. A more normal approach would not be to use a combination in/out variable, but instead take in double and return double. No pointers or references required.
Doing otherwise might be a premature optimization and sometimes is actually counter-productive. It is quite possible that writing code more focused on clarity might allow the compiler to optimize it better. Hard to know without real-world testing with the whole program end-to-end.
Indeed, a double is hardly more work to pass around than a pointer, and compilers often have problems with optimizing pointers/references. You could try timing it using pass-by-reference and pass-by-value, and possibly add "inline" to hint to the compiler that it might be a good idea to inline this particular function.
In any case, excellent work on cleaning up this kind of stuff.
On 30-1-2013 9:36, Jasper van de Gronde wrote:
On 29-01-13 23:47, Jon Cruz wrote:
On Jan 30, 2013, at 9:19 AM, Johan Engelen wrote: ...
Alright, this method should not do a NULL check for performance reasons (I assume). But, shouldn't we use a reference in this case then? To signal clearly that the method does not accept NULL pointers without crashing: ...
I'm trying to reduce the number of crashes due to simple NULL pointer crashes. I find such bugs satisfying to fix because it takes really only 1 minute to locate the bug and 1 minute to fix and commit it, but........
Yes, that does seem reasonable.
However... I would wonder about performance. A more normal approach would not be to use a combination in/out variable, but instead take in double and return double. No pointers or references required.
Doing otherwise might be a premature optimization and sometimes is actually counter-productive. It is quite possible that writing code more focused on clarity might allow the compiler to optimize it better. Hard to know without real-world testing with the whole program end-to-end.
Indeed, a double is hardly more work to pass around than a pointer, and compilers often have problems with optimizing pointers/references. You could try timing it using pass-by-reference and pass-by-value, and possibly add "inline" to hint to the compiler that it might be a good idea to inline this particular function.
See r12077. Don't even know how to properly test whether I broke anything, so... :-)
In any case, excellent work on cleaning up this kind of stuff.
I hope everybody takes note and tries to improve their commits!
Cheers, Johan
participants (3)
-
Jasper van de Gronde -
Johan Engelen -
Jon Cruz