Bug: check-before-leaping idiom is harmful to performance

[Robert Collins]

We have a check-before-leaping idiom in a number of places in the knit
code.

e.g.:
def _get_content_maps(self, version_ids):
    """Produce maps of text and KnitContents

    :return: (text_map, content_map) where text_map contains the texts
for
    the requested versions and content_map contains the KnitContents.
    Both dicts take version_ids as their keys.
    """
    for version_id in version_ids:
        if not self.has_version(version_id):
            raise RevisionNotPresent(version_id, self.filename)
    ...

This is very wasteful: we're going to have to access the data for each
version_id anyway, so rather than checking that they all exist, it is
more efficient to try and when we find a missing one, *then* error.

In terms of wastefulness - this one loop is 10% of the time to do 'bzr
checkout --lightweight' with a Pack repository.

While its true that this may have much more to do with some
inefficiencies in the index layer, it causes us to pass over the index
twice, and thats bad because if we have a large index that may cause
index cache misses - and they will have a much larger cost.

I'm filing this as a bug in malone, because I think it would be sensible
to do an audit for such things and fix them in one fowl swoop, and bugs
are IMO a good way to get tasks visible to contributors.

 affects /products/bzr
 status triaged
 importance high

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070815/b9f4ea55/attachment.pgp