Re: [PATCH v4 5/6] remote: die if branch is not found in repository

[Glen Choo <chooglen@google.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

>> To prevent misuse, add a die_on_missing_branch() helper function that
>> dies if a given branch is not from a given repository. Speed up the
>> existence check by using a new branches_hash hashmap to remote_state,
>> and use the hashmap to remove the branch array iteration in
>> make_branch().
> This makes sense, but how often would this be called?

This affects most of remote.h branches API (branch_get(),
branch_get_upstream(), branch_get_push()). From what I can tell, I don't
think these are called very frequently.

> Couldn't we just iterate over the array (instead of making a hashmap)?
> If speed is important, I think we could just sort the array and do a
> binary search.

The primary reason I used a hashmap is to be consistent with struct
remote (which also uses a hashmap). One possible argument in your favor
is that remotes are often looked up by name often (and justify the
hashmap), whereas branches are not looked up by name as often (and don't
justify a hashmap).

I say _justify_, but I don't see significant drawbacks to using a
hashmap here. I suspect that there is an advantage to binary search that
you haven't made explicit yet? Could you share your thought process to
help inform the decision?