On Sat, 13 Oct 2007 15:27:06 -0300, "Gustavo Sverzut Barbieri" <[EMAIL PROTECTED]> wrote :
> On 10/13/07, Simon TRENY <[EMAIL PROTECTED]> wrote: > > On Wed, 26 Sep 2007 00:58:05 -0400 (EDT), > > Enlightenment CVS <[EMAIL PROTECTED]> wrote : > > > > > Enlightenment CVS committal > > > > > > Author : barbieri > > > Project : e17 > > > Module : libs/etk > > > > > > Dir : e17/libs/etk/src/lib > > > > > > Log Message: > > > Structures fields reorder to avoid holes and save memory. > > > > > > Have all Etk_Bool inside structures to be a bitfield, reorder > > > fields to provide better packing. > > > > > > Packaging was aided by pahole 1.0 on x86 > > > (http://git.kernel.org/?p=linux/kernel/git/acme/pahole.git) > > > > > > Tested with etk_test and edje_viewer, both still work. > > > > > > PS: bugs may appear due Etk_Bool size change, values like 2 (10b) > > > will now be evaluated as false. That was already a bug, just > > > being exposed now, the fix is easy: use !! (double negative) > > > before the value, example: "visible = obj_ptr;" becomes "visible > > > = !!obj_ptr;". > > > > Can I ask you how many bytes per widget do you gain with such a > > change? I don't really expect it to be more than 300b per widget. > > Since in an Etk app, you'll most probably never have more than 300 > > widgets, you'll gain 100kb max, and I'm being **VERY** generous > > here! :) And I don't think 100kb is really important nowadays, even > > on embedded devices (whose apps will never have 300 widgets > > anyway...). > > > > So, if you don't mind, I'm going to revert this change, as it > > introduces code changes that I personally find really ugly (such > > as "visible = !!obj_ptr;"...) > > > > And, it would have been great if this change had been discussed on > > the dev ML before being committed. I know I haven't been really > > active these last weeks, I've been quite busy with school and other > > projects, but I'd still like to have the opportunity to give my > > opinion before such a change happens. > > > > More generally, there has been a lot of important changes recently > > in Etk internals, some of them are great, but there also have been > > changes that I really don't like. I would have appreciated if you > > (i.e. developers who made these changes) had discussed them > > publicly and *waited* for my response before committing them. > > Ok, I feel a bit guilt here since I did most of the changes you > disliked, sorry about that, but they all have a good reason. > > Avoiding holes in structs is not just about saving space, is fitting > things better into cache lines, is using memory better and believe me > or not, it makes difference. Ok, I can't test so I trust you on that. Using "unsigned int bool : 1" for booleans is not a bad idea, I just don't like some of the consequences it has brought (especially the "!!" thing) > > Also, forcing boolean fields to be 1 or 0 (ETK_TRUE or ETK_FALSE) > makes sense or we still have bugs depending on how software is > developed. Today it fails if you use "2" (10b) as "true", but it would > also fail if you compared with ETK_TRUE, so that _IS_ a bug. If you > dislike !!var, then use something else, but make sure it's 1 > (ETK_TRUE) or 0 (ETK_FALSE) if you tagged them as Etk_Bool. Yes, you made a good point here. Maybe we could then just have a macro called TO_BOOL() that does the integer-to-boolean cast. So there will be no "!!" in the code anymore. > > This specific change was not discussed in open, but others, like > Etk_Signal, were and at least CodeWarrior said: go for it. Before we > were keeping those changes in GIT branches at http://staff.get-e.org > and CodeWarrior wisely requested us to keep these changes in CVS to > avoid fragmentation. > > We (INdT) choose ETK over EWL because it was easier to get our > requests done and accepted, but we said (at least to CodeWarrior) that > we wanted some things changed. We really want to cooperate and build a > better platform together, but if you don't want these things in, we'd > have to keep a separate branch/fork with changes you still haven't > accepted... this will be painful for everybody. :-( > > So, when will you be back online? and which development approach would > you like (separate code bases or development in CVS)? Yes, having several branches is not a good idea imho. Especially since you (devs at INdT and other Etk devs) did an amazing work recently and since I think we share the same common goals in Etk, and it would be a shame to spread our efforts. In order to avoid having this kind of situation again, I think we could set 2 rules that should be applied for *important* changes: 1st - Always discuss major changes publicly (i.e. on the Dev ML, not only on IRC). If I don't give my opinion 7 days after the beginning of the conversation, you can commit the changes, it would be entirely my fault if I'm not happy with them :) I know I didn't participate to the Etk_Signal dicussion thread, so you were right to commit it. And actually, I even start to like those changes. :) 2nd - Don't forget to CC me when you add a patch to bugzilla (there has been a couple of patches that I missed because I wasn't CCed on these) Of course that doesn't apply for small changes, it just should be respected for big internal changes that affects a lot of files. You can still commit most of your changes directly. Regards, Simon TRENY <MoOm> > ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
