On Thu, Jul 21, 2005 at 08:06:53PM +0300, Sami Liedes wrote: > Package: apt > Version: 0.6.38 > Tags: patch > Severity: wishlist > > Hi, > > I profiled aptitude a bit and found out that most of its slowness(*) > is due to apt (which shouldn't be that much of a surprise, aptitude > being mostly only a frontend for apt). Here's a patch that speeds up > some things quite a bit, especially with some optimizations to > aptitude too (which I will submit separately soon).
I've put your patch into arch, on the branch apt--string-copies--0. The archive is at http://people.debian.org/~mdz/arch/[EMAIL PROTECTED]/ as always. The one bit that I'm uncertain about, after a quick read, is the change to stringcasecmp(). I'm not sure that it's guaranteed that the locale settings will always be the same in every context. I think it's probably safe for the existing code, but risks unexpected behavior. Did you profile any of these changes separately? What were the objective results from your changes as a whole? mdz Integral_: I have ~100 unread in that particular folder Integral_ oh Integral_ it's bug #319377 Integral_ I skimmed over it and it looks like it's mostly changes to avoid string copies. mdz Integral_: hmm, I wonder why it is against such an old version Integral_ I was thinking that it might be simpler to use a string class that handles immutable strings with refcounting -- it would be less efficient but it would also be harder to re-introduce string copies by accident Integral_ mdz: I think it might be a typo, the Version: header says 0.6.38 mdz Integral_: libstdc++ still doesn't include a useful string class, does it? Integral_ mdz: well, it provides one that copies like crazy every time you use it mdz Integral_: the patch mostly looks reasonable; I'm not sure about the stringcasecmp change wrt locale correctness Integral_ mdz: Yeah, I think tolower() is allowed to change its behavior in other locales. Integral_ Or toupper() as the case may be mdz Integral_: it might not be an issue depending on where stringcasecmp is called; I haven't looked mdz hmm...surely two .push_back()s shouldn't be significantly different, performance-wise, from += (null-terminated) Integral_ mdz: grep suggests it's used for version matching, parsing the policy file, and comparing package names. Integral_ mdz: OTOH, it shows up in strutl.h, so if you're really unlucky, people are using it in the wild Integral_ mdz: yes, some of the idioms there are weird. I'd also think that insert(s.end(), s2.begin(), s2.end()) should be the same as s += s2. Integral_ mdz: I think he changed += to push_back in some places because he changed a string to a vector Integral_ which seems pointless to me Integral_ unless the g++ implementation of std::string is truly atrocious mdz Integral_: does that patch actually apply for you? it seems corrupt mdz rediff gets it to apply, but I'm not sure tha tit's correct mdz that it's Integral_ mdz: I haven't tried to apply it, actually Integral_ mdz: it applies everywhere but init.cc for me mdz Integral_: likewise mdz rediff | patch works Integral_ hm mvo mdz, Integral_: did you measured the perforance difference? mvo (resulting from the patch) mdz Integral_: mind if I copy our conversation to the BTS? mdz mvo: no, I've done nothing but make it apply to get it into arch Integral_ mdz: go ahead -- - mdz -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
