On Mon, Jan 7, 2019 at 10:20 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> When seeing a tag or note ref, by definition that's not something we
> can have checked out in any worktree. I wonder if it is worth to
> optimize further by omitting this lookup when ref is not a local
> branch?
>
> IOW, with a typical number of worktrees and refs, how costly would ...
>
> > + hashmap_entry_init(&entry, strhash(ref->refname));
> > + lookup_result = hashmap_get(&ref_to_worktree_map, &entry, ref->refname);
>
> ... this sequence of calls be.
>
It certainly wouldn't be free. Every check would compute the hash of
the refname and do a lookup into the hash table. If the bucket it
looked up was empty that'd be it, but if it were non-empty a few more
comparisons would happen.
I think avoiding this would be check, we can simply check ref->kind ==
FILTER_REFS_BRANCHES ahead of calling into get_worktree_path and
provide an empty string otherwise.
> > free_array_item(array->items[i]);
> > FREE_AND_NULL(array->items);
> > array->nr = array->alloc = 0;
> > + if (worktrees)
> > + {
>
>
> What's the point of ref_array_clear()? ... If that
> is the case, then the added code breaks that hope, as it leaves a
> dangling pointer in the worktrees variable.
>
Discussion of the point of ref_array_clear would be out of scope of
this change, but your point is well taken that setting worktrees to
NULL would be consistent with the rest of the function. Will
implement.
