On 4 Dec, Sven Neumann wrote:
> Using them for error reporting is definitely a bad idea. Using a
> negative value to indicate that a value has not been set and needs to
> be computed is IMO a reasonable usage.
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.
> 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.
> 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.
> I agree about the return value thing, but I doubt the use of signed or
> unsigned makes up for any noticeable speed difference expect under
> very rare conditions. Before any integer is changed to an unsigned
> value, a benchmark should proove that it really is an optimization and
> all code using the value needs to be reviewed carefully.
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.
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.
--
Servus,
Daniel
