kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

Hi,

I hope I don't sound ranty - but really, I think we need to put in place some
policy about well-meaning but ineffective optimizations. They're one of the
quickest ways to screw up maintainability and introduce the possibility of
errors.

I strongly suspect 6711 "Optimize VECTOR2::Rotation for 0, 90, 180 and -90
degrees by avoiding time consumming calculations." was not profiled. I profiled
it. Here are some results, rotating all vectors from (-10000,-10000) to
(10000,10000) by various angles:

TESTING NEW FUNCTION, INT
Total time, common rotations only: 927787 us
Total time elapsed: 43971528 us

TESTING NEW FUNCTION, DBL
Total time, common rotations only: 1043882 us
Total time elapsed: 43678411 us

TESTING OLD FUNCTION, INT
Total time, common rotations only: 964067 us
Total time elapsed: 43955722 us

TESTING OLD FUNCTION, DBL
Total time, common rotations only: 1075902 us
Total time elapsed: 43649236 us


My test code is here: https://github.com/cpavlina/kivectest


For the common (0, 90, 180, -90) rotations that this was supposed to optimize,
this sped things up by _four percent_ for integers and three percent for
doubles. For uncommon rotations it made things _worse_ by 0.03% for ints and
0.06% for doubles.

These are very, very small numbers. Honestly, is it worth the added code
complexity? The added chance of bugs? (None here that I can see, but it's a
risk we take every time we allow something like that). For a 3% improvement in
some code that represents a quite small fraction of KiCad's total CPU time.

My personal opinion is that for changes that are pure optimization with no
_improvement_ to the code style/readability/maintainability, we should require
that the developer actually show that there is a problem or that the given area
actually contributes significantly to the overall performance, then demonstrate
_with numbers_ that the new code offers a significant improvement in
efficiency. Generally we should be looking at things that are O(big) and used
frequently, not simple vector rotations that can run over nine million function
calls per second!

-- 
Chris