[MERGE] reduce list copies during single text extraction

Ian Clatworthy ian.clatworthy at internode.on.net
Mon Oct 22 09:54:27 BST 2007


Robert Collins wrote:
> This avoids list copies during single text extraction. Matt Nordhoff
> reported that commit was taking minutes, and a callgrind report he
> mailed me showed this as a significant factor. I suggested a cheap way
> to remove 2 of these copies on IRC which saved about 30% of commit. This
> patch removes those two and a third (the one in _apply_delta) for
> hopefully a 45% saving ;).

Very good catch.

bb: tweak

> +    def apply_delta(self, delta, new_version_id):
> +        """Apply delta to the this object to become new_version_id."""

s/the this object/this object/.
3 times for that. :-)

I would have put the code from the old _apply_delta() into the base
implementation, eliminated one derived implementation and made the other
2 lines long. It's not a big deal in this case though because the
algorithm is simple and has next to zero chance of ever needing to change.

It also isn't really obvious what you're gaining in practice by setting
version_id for the PlainKnit case (given it wasn't done before?) though
it doesn't hurt and is conceptually good I think.

Looking forward, I really think that getting content for a single
version ought to be a cut-down special case with it's own routine.
_get_content_maps now builds quite a lot of data structures that aren't
required and has a lot of logic (if branches) that no longer applies.
For now though, this is a big win as is.

Ian C.



More information about the bazaar mailing list