On Wed, 17 Nov 2010 14:28:26 -0500, Austin Clements <amdragon@MIT.EDU> wrote: > This reduces thread search's 1+2t Xapian queries (where t is the > number of matched threads) to 1+t queries and constructs exactly one > notmuch_message_t for each message instead of 2 to 3. Fantastic stuff, Austin! I've merged this now, (sorry it took me a while to get to it). One of the reasons I didn't merge it immediately is that I wanted to ensure that I understood the original author-ordering bug. Basically, I'm inherently uncomfortable with a performance optimization that fixes a bug as a side effect, (unless we understand that very well). So what I pushed actually adds the bug fix first, so that the performance optimization makes no change at all to the test suite. That feels better to me, (even though it simply demonstrated conclusively that the bug was in a piece of code that was eliminated by the optimization). Anyway, in a quick reading of the code, the only little thing I saw was: > + size_t count = (bound + sizeof (doc_ids->bitmap[0]) - 1) / > + sizeof (doc_ids->bitmap[0]); Which would look better to my eyes with a 1 factored out of the division: size_t count = 1 + (bound - 1) / sizeof (doc_ids->bitmap[0]); And the repeated use of "sizeof (doc_ids->bitmap[0])" could maybe do with a macro for better legibility. Though it would be an evil macro if it didn't accept an argument, and it wouldn't be much shorter if it did. So maybe it's fine as-is. Thanks for the optimization. Now all we need is a little notmuch benchmark so that I can be sure not to regress any performance work with my sloppy coding! -Carl -- carl.d.worth@intel.com