Re: [Gimp-developer] Re: your so called optimizations and why we don't like them

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

degger@xxxxxxx writes:

> IMHO not because you're abusing the real value for errors and thus one
> variable for 2 purposes which is a bad idea and using signed integers is
> dragging down performance.
> 
> It is also a bad idea to use signed integers for most loops for example;
> unsigned int i;
> for (i = 0; i < width; i++)
>   foo ();
> 
> tends to be more efficient in some cases (depending on the use of i)
> then when categorily using signed ints.

this is not true. Please stop spreading this nonsense about unsigned
integers being more performant than signed ones. Go and write yourself
a simple benchmark for the code you mentioned above and you will
notice that it doesn't make any difference at all.

> > you also removed a good bunch of debugging code that might still be
> > needed (Kelly is working on this code) and "unsignified" lots of
> > integers, which is why I choose to revert the changes in this file in
> > a whole.
> 
> The debugging cruft is pretty worthless, something to be added if really
> needed.

yes, but it is not nice to remove it while Kelly was working on this
part of the code.

> > I do hope you will post the tile_manager_get_tile_num()
> > part of your change to the list.
> 
> Actually it makes only sense when using unsigned variables because
> otherwise the optimisation is mostly gone. If we can agree that nrows
> and ncols are unsigned then I'm willing to post the patch here....
>  
> > If code makes assumptions about parameters (like for example assuming
> > that width is > 0), there has to be code like g_return_if_fail() to
> > check this assumption. Defining width as an unsigned integer would
> > also guarantee this, but it unneededly introduces a possibility for a
> > bug if width is ever used in a subtraction.
> 
> Not in the subtraction itself, only if the destination is also unsigned
> and if one expects a positive result and gets a negative one there's a
> bug in the code anyway.

the side effects of unsigned integers are not what people are used to
think about when designing an algorithm. You are changing the
mathematical base in an unneeded and hardly foreseeable way. Code that
looks correct and used to work fine, suddenly starts to behave
wrong. I doubt that you've checked each and every usage of the
variables you changed lately.

IMO using unsigned integers is a concept from the stone-age of
programming when CPUs and compilers used to be crap and the use of
unsigned values gave a real speed gain.

> FWIW I've converted lots of parameters and variables in paint-funcs to
> unsigned and I've experienced a 2,5% object code reduction in this part
> of the GIMP and since the calculations were simplified by the compiler
> (proven by assembly examination) it's for sure also faster though it's
> quite hard to profile this code since there's no benchmark generating
> reproducible results.

what are you aiming for, GIMP-embedded? You are speaking about ~1.3 KB
of object size reduction. This is ridiculous and there are certainly
other parts of The GIMP that can be made much slimmer with a less
error-prone approach. I also doubt that the code size reduction or
speed improvement originates in the use of unsigned variables. It
would surprise me very much if it would make any difference for the
compiler. Signed and unsigned variables are treated the same in almost
all situations.

If you are about to optimize parts of the GIMP, the first thing you
should do is write a test suite and benchmarking facility for this
part of The GIMP. No optimization should be allowed without such a
test suite.

> I still think GIMP is too fat and the code in parts too ugly. Though my
> changes in general just show small improvements I usually also fix
> stylistic problems and find unoptimal code and bogosity during the
> audits. Having a clear view what a function does and which parameters
> it expects is pretty important and correctly typing their arguments
> is a good step forward in achieving quality code which always runs
> at maximum speed.

Well, going through the code like a berserk doesn't really help here.
First, it is way too early to start optimizing the code in a whole.
Then, only code that is finished, working and stable should be
optimized to make it possible to catch errors introduced by the
optimizations.  Last, only code that showed up in a profile is worth
to be optimized.

What needs to be done at the moment is to give the GIMP core a
well-defined structure, to document and to debug it. A few small
encapsulated areas like the paint-funcs can be optimized now and if
benchmarks proove that the use of unsigned variables speeds things up,
then use them there for god's sake. But please, stop going over other
peoples code that you don't understand and stop applying your so
called optimizations to it. Thank you.


Salut, Sven





[Index of Archives]     [Video For Linux]     [Photo]     [Yosemite News]     [gtk]     [GIMP for Windows]     [KDE]     [GEGL]     [Gimp's Home]     [Gimp on GUI]     [Gimp on Windows]     [Steve's Art]

  Powered by Linux