Hi, degger@xxxxxxx writes: > But you also do read my mails, do you? And I said clearly that it might > make a difference in larger functions not that it necessarily betters > anything. but you continue to state that it makes the code cleaner which it clearly doesn't. > Example? Ok, here you go, I subsituted an gint which is used for > a clearly positive range (1; length) by an guint. This is an arbitrary > example and doesn't give to much of a benefit though the function is > still simple. > > --- paint-funcs.c.orig Thu Nov 29 14:17:47 2001 > +++ paint-funcs.c Tue Dec 4 21:53:49 2001 > @@ -343,7 +343,8 @@ > gdouble sigma2; > gdouble l; > gint temp; > - gint i, n; > + guint i; > + gint n; it gives a small improvement and is probably a valid optimization in this place. The difference to the changes I reverted is that this is a very local change. The function is small and the variable is local. It is easy to decide that using an unsigned integer here does not have any unwanted side effects. Your changes however did affect mostly headers. This means the values are used in a lot of places and it is almost impossible to check them all. Even if you would check them all, a future change to the code might introduce a bug because the one who did the change didn't think about the unsignedness of the variable (which is defined somewhere else). Since the code is very much in flux right now, a lot of these changes are going to happen in the future. It is definitely too early to do such global optimizations and I even vote for never doing them. Using unsigned variables locally in parts of the GIMP core that are executed frequently is still a valid option. > > I haven't said that and you will have noticed that we've added lots of > > consts to the code recently. > > And this is a good thing. It might break stuff exactly like the > "unsigned" case but it will lead to better code and is much cleaner. no, it breaks things in a very different manner: the compiler will now warns us if we try to modify values that we aren't supposed to modify. Thus we catch bugs at compile time. The compiler will however not warn us if we use unsigned variables in a way that might cause the value to become negative. The result here will be a mis-behaving application with no indication of where we should look for the problem. In the worst case the problem only becomes apparent under rare circumstances and is not even caught during the development process. > I wouldn't have touched the UI code if it was more seperate from the > rest. the directories that contain pure UI code are called gui and widgets. Is that not simple enough? The point however is not to disallow optimizations in the UI and allow them everywhere else. Most parts of the core are unfinished yet and shouldn't be optimized at this stage. > > 1. Move files into subdirectories that define subsystems and reduce > > the dependencies between these subsystems. > > 2. Clean up the code so it uses a common coding-style. > > 3. Make the code bug-free (this should involve writing test suites > > for the various subsystems). > > 4. Profile it. > > 5. Optimize it. > > > > Since we aren't even done with parts 1 and 2; can't you understand > > that we'd prefer not to start with 5 yet?! > > Sure, but the changes were also part of your point 2, and it's never > to early to work on cleanliness. we obviously disagree on code cleanliness. IMO the use of unsigned variables does not make the code any cleaner. It is solely part of an optimization process and optimization has to come late and should be applied with lots of care. Salut, Sven
