I was a vfx render engine engenieer for years, and I can tell you that it is really easy to mess up things with "optimization". I didnt check the code for this particular case, but a simple test with a loop is not enough to tell you if an optimization means something.
- It can make the funcion size bigger, more code cache misses -> slower code - it can create a branch prediction miss -> slower code - it can force sse instructions with more load/store -> slower code - it can use a temporal variable, forcing everything that was working in registers, to move now to memory -> slower code - it can be fast on intel, but horrible in ARM -> slower code Normally, it is easier to optimize the algorithm and the data structures (specially data structures) ᐧ *Javier Loureiro Varela* On Tue, Apr 26, 2016 at 5:23 PM, Chris Pavlina <pavlina.ch...@gmail.com> wrote: > Okay, the numbers aren't quite so nice. I posted the wrong ones and now > have > egg on my face. That's what I get for playing around with git too much. > > I still stand by what I said about needing to show improvement, though. > While > the new code *does* improve things by about 15% on my computer when I run > the > right code... I still notice *no* difference in KiCad. > > > On Tue, Apr 26, 2016 at 11:07:52AM -0400, Chris Pavlina wrote: > > 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 > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp >
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp