13 Nov
2007
13 Nov
'07
9:12 p.m.
On Nov 13, 2007 2:01 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote: > > On Nov 13, 2007 12:47 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote: > > > Any notable optimization worth the hack. I disagree, just as Aaron stated below. The biggest reason? This is code that crosses the libalpm/pacman barrier. When that happens, interfaces should be as clean as possible. > > I just need to comment here that I'd prefer we _not_ do this in > > pacman. "Any optimization is worth it" is a REALLY bad mantra when it > > comes to code, in my experience. Code should be clean and elegant > > _first_ and as optimized as possible _second_. So if optimizations > > break code clarity and add unnecessary stuff for a 0.1 second > > improvement, it's not worth it. > > > > Sure large improvements are worth a loss in code clarity, but, as Dan > > actually proved with the timings, there's very little room for > > improvement here. > > But you should see, that your reasoning is simply impossible to keep, > because "clean code" and "elegance" are _subjetive_ things: Sure they are. And unfortunately since Aaron and I are the committers to the projects.al.org GIT tree, you will be held to our definitions of clean code and elegance. And I'm not trying to sound nasty here- I'm just laying out the facts. > -_imho_ sync.c is not elegant at all I don't believe we have ever disagreed with you here, so no need to fan the flames on this. Aaron and I didn't write this code, we've inherited it. At the same time, it works, and a monster patch that I can't digest isn't the solution to fixing it unless there is a great deal of discussion on how the new code works. > -_imho_ your alpm_list patch (first prev == tail) is not elegant (I would have > preferred alpm_list_add_first) A few issues here. 1. This is completely inside libalpm- there is no noticeable interface change to the "end user" of libalpm. 2. This is a huge performance increase. Thus, we can sacrifice some elegance for what we did. And it was commented well. 3. alpm_list_add_first would not help when it comes to printing the filelists of all the packages, etc. It would print them all in reverse order, which would confuse users. > -_imho_ a function which we could _radically_ speed-up with a 'first' > parameter and an 'if(first) return ...' line, is not elegant Hmm? Please keep this constructive if you (anyone on the list) respond to this thread. We all want to work on pacman, hear bitching about what "elegant" means. -Dan