Code review comment for lp:~ian-clatworthy/bzr/faster-diff2

Martin Pool (mbp) wrote :

I'm really not sure about the cache part of this:

Folding both file_id and path_id into a single string is potentially (in obscure cases) buggy, and at least seems unnecessary; using a tuple may even be faster. Also I'm seriously concerned that the cache is not invalidated in the dirstate is mutated, and would question whether it's a good idea to put a cache of unlimited duration in at this level. (I wonder what John thinks.) Could you store it at the diff level?

review: Disapprove

« Back to merge proposal