This is a good point. A test like this isn't particularly enlightening, which is why I didn't bother with updated numbers after realizing my mistake. They don't tell much.
Really the important bit is whether things are improved *in the operating code*, and as far as I can tell, they are not - unless the original developer has a test case that brings out the issue. On Tue, Apr 26, 2016 at 05:43:16PM +0200, Javier Loureiro Varela wrote: > 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