kicad-developers team mailing list archive

[Javier Loureiro Varela <derethor@xxxxxxxxx>]

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
>