Thread (28 messages) flat view 28 messages, 3 authors, 2021-11-16

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help