This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [setup PATCH] Improve Category column (Take 2)


On Sun, 2002-09-22 at 22:13, Max Bowsher wrote:
> Robert Collins wrote:
> > Much better. Please supply as a attachment, along with a changelog.
> 
> Will do.

Cool, Thanks.
 
> OK. It was a (not so good) attempt at a performance optimization.

Done too early :].
 
> > > +  set<String, String::caseless>::const_iterator all =
> categories.find("All");
> > this is also not needed, and if we were using something that supports
> > multiple entries (say a vector) with the same key, would give incorrect
> > results.
> 
> My gut feeling was that an iterator should compare quicker than a string. I
> can't see any ill effects to keeping this one. Multiple categories with the same
> name aren't going to happen :-)

It makes assumptions about the type of container we are iterating over,
that a change in code elsewhere will break. That makes it fragile
(relatively speaking). I'd rather have robust code. In terms of speed,
we can cache the entire results and rebuild the readableList on changes
to the category, so we can eliminate the method call as having any inner
loop performance impact at all.
 
> I deliberately tried to minimize the number of operations done in the loop.
> Convention vs. a (possibly insignificant) performance gain. Your call.

I apprecaite that :}. Mm, I think that what I'm suggesting is actually
less operations, after inlining. All the same, both your code and mine
does many string operations, some of them with ", ", and that will
probably be the greatest overhead. 
 
> Advance warning: I've also got a patch for dumpAndList in
> IniDBBuilderPackage.cc, which uses the same 'break in middle of while(true)
> iteration' technique to achive the same goal (no delimeter at end of list). The
> end result is that and list dumps are all on one line, greatly reducing number
> of log lines, and also (I think) much more readable. It formats and lists like
> this: "foo & baz & baz". If or lists are used (currently they are not), it
> formats them as: "foo | ( bar & baz )".
> 
> So, if you have a preference for coding style, tell me now, and I will apply it
> to that patch too.

I value human clarity over speed tweaks. Large performance gains usually
come from algorithmuc improvements. Also, we should profile before
picking a particular area for concern.

Rob

Attachment: signature.asc
Description: This is a digitally signed message part


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]