kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

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.chris@xxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> >