Code review comment for lp:~ian-clatworthy/bzr/log-show-custom

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

John A Meinel wrote:
> I think it would be worthwhile to figure out why this is so expensive.
> In general, it seems like it should just be a single dict lookup to see
> if a given property exists, which doesn't seem particularly expensive.
>
> Is it just an inefficient check in bzr-svn, or is it something about the
> API that makes it inefficient? (Such as passing the revision id, rather
> than the Revision object, etc.)
>
It's slow for 2 reasons:

1. a custom property handler is *always* registered for calculating
    foreign properties. So simply testing if the registry is empty isn't
   good enough to short-circuit the normal processing loop.

2. bzrlib/foreign.show_foreign_properties() tries to pull apart
    *every* revision-id to see if it came from a foreign source.

Over 250K revisions, these effects add up with the second one being
particularly expensive. It would be nice if there was a single check we
could do (per branch or repo say) to decide whether to do the above but
I can't see how we can, given the possibility that just a few revisions
here or there (in a merge say) came from a foreign source. Hence my
suggestion that they be switched off by default, so only users
interested in them pay the cost.

BTW, hg has a nice feature where options can be configured per command
per branch (by adding sections to their branch-specific config file). I
think that sort of flexibility is useful in case like this (so that
those working with foreign branches can enable --show-custom by default
when they care without going as far as a per user alias). Just a thought.

Ian C.

« Back to merge proposal