Re: [PATCH v4 5/6] remote: die if branch is not found in repository
From: Jonathan Tan <hidden>
Date: 2021-11-16 17:45:58
Glen Choo [off-list ref] writes:
quoted
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?
The main drawback is that branches are now stored in 2 ways - as the "branches" array in struct remote_state and as this new "branches_hash". I think we should avoid storing the same data twice unless we really need to, and I don't think there is a need here. As for hashmap vs array (say, if we were thinking of removing the array and putting in a hashmap instead), I would still prefer the array (sorted if needed) just for the simplicity, but I wouldn't feel as strongly about this since there is no duplication here.