symforce icon indicating copy to clipboard operation
symforce copied to clipboard

Idea: `PerformanceWarning`

Open aaron-skydio opened this issue 2 years ago • 2 comments

Some thoughts from discussing this:

  • It might be nice to have a warning system for “things that are bad for performance”. In particular, the first example I’ve come across is std::pow(x, y) where y is a symbol at codegen time, but is a constant parameter at runtime (in particular an integer, in this case and the worst case 1.0). I can imagine maybe also wanting warnings for integer powers if fast-math isn’t on? Although maybe our codegen should handle this, or assume fast-math.
  • Probably if pow(x, y) is the only example, this doesn’t make sense? Not sure if we have other examples
  • Misc thoughts:
    • Docs page would be good
      • probably want this regardless
    • Compilers like ispc and numba have things like this
    • Want to be able to configure as warning, or error, or ignored
      • Need to do this per subexpression for big things like OptimizationProblem that encompass lots of expressions / residuals
      • Extra attribute per expression?
      • Code printer probably checks

aaron-skydio avatar Jan 03 '24 22:01 aaron-skydio

I noticed my generated code has a lot of std::pow calls where y = 2 is known at codegen time. Is there anything I can do to make symforce emit something like _tmp2 = _tmp1 * _tmp1 instead?

jpreiss avatar May 17 '24 22:05 jpreiss

For a constant exponent of 2 known at codegen time, your compiler should optimize the std::pow to a multiply, so we don't bother special casing that to generate a multiply - the logic for which powers we special-case is here: https://github.com/symforce-org/symforce/blob/main/symforce/codegen/backends/cpp/cpp_code_printer.py#L68

If you have a compiler where this doesn't get optimized and actually results in a function call or something else that's not just a multiplication instruction that would be good to know though and we can also special-case that to generate a multiplication

aaron-skydio avatar May 17 '24 23:05 aaron-skydio