In find_non_local_tags() helper function (used to implement the
"follow tags"), we use string_list_has_string() on two string lists
as a way to see if a refname has already been processed, etc.
All this code predates more modern in-core lookup API like hashmap;
replace them with two hashmaps and one string list---the hashmaps
are used for look-ups and the string list is to keep the order of
items in the returned result stable (which is the only single thing
hashmap does worse than lookups on string-list).
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
* Just to remind ourselves that we talked about getting rid of
string-list used as look-up tables by replacing them with hashmap
but haven't seen much effort expended on it. I think this is my
first semi-serious use of hashmap, and the code is expected to be
full of newbie mistakes, inefficiencies and ignorance of idioms;
pointing out any of which is much appreciated.
builtin/fetch.c | 116 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 90 insertions(+), 26 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 0696abfc2a..5d1fd8dd49 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -246,16 +246,73 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
return 0;
}
+struct refname_hash_entry {
+ struct hashmap_entry ent; /* must be the first member */
+ struct object_id oid;
+ char refname[FLEX_ARRAY];
+};
+
+static int refname_hash_entry_cmp(const void *hashmap_cmp_fn_data,
+ const struct refname_hash_entry *e1,
+ const struct refname_hash_entry *e2,
+ const void *keydata)
+{
+ return strcmp(e1->refname, keydata ? keydata : e2->refname);
+}
+
+static struct refname_hash_entry *refname_hash_add(struct hashmap *map,
+ const char *refname,
+ const struct object_id *oid)
+{
+ struct refname_hash_entry *ent;
+ size_t len = strlen(refname);
+
+ FLEX_ALLOC_MEM(ent, refname, refname, len);
+ hashmap_entry_init(ent, memhash(refname, len));
+ oidcpy(&ent->oid, oid);
+ hashmap_add(map, ent);
+ return ent;
+}
+
+static int add_one_refname(const char *refname,
+ const struct object_id *oid,
+ int flag, void *cbdata)
+{
+ struct hashmap *refname_map = cbdata;
+
+ (void) refname_hash_add(refname_map, refname, oid);
+ return 0;
+}
+
+static void refname_hash_init(struct hashmap *map)
+{
+ hashmap_init(map, (hashmap_cmp_fn)refname_hash_entry_cmp, NULL, 0);
+}
+
+static int refname_hash_exists(struct hashmap *map, const char *refname)
+{
+ struct hashmap_entry key;
+ size_t len = strlen(refname);
+ hashmap_entry_init(&key, memhash(refname, len));
+
+ return !!hashmap_get(map, &key, refname);
+}
+
static void find_non_local_tags(const struct ref *refs,
struct ref **head,
struct ref ***tail)
{
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
- struct string_list remote_refs = STRING_LIST_INIT_NODUP;
+ struct hashmap existing_refs;
+ struct hashmap remote_refs;
+ struct string_list remote_refs_list = STRING_LIST_INIT_NODUP;
+ struct string_list_item *remote_ref_item;
const struct ref *ref;
- struct string_list_item *item = NULL;
+ struct refname_hash_entry *item = NULL;
- for_each_ref(add_existing, &existing_refs);
+ refname_hash_init(&existing_refs);
+ refname_hash_init(&remote_refs);
+
+ for_each_ref(add_one_refname, &existing_refs);
for (ref = refs; ref; ref = ref->next) {
if (!starts_with(ref->name, "refs/tags/"))
continue;
@@ -271,10 +328,10 @@ static void find_non_local_tags(const struct ref *refs,
!has_object_file_with_flags(&ref->old_oid,
OBJECT_INFO_QUICK) &&
!will_fetch(head, ref->old_oid.hash) &&
- !has_sha1_file_with_flags(item->util,
+ !has_sha1_file_with_flags(item->oid.hash,
OBJECT_INFO_QUICK) &&
- !will_fetch(head, item->util))
- item->util = NULL;
+ !will_fetch(head, item->oid.hash))
+ oidclr(&item->oid);
item = NULL;
continue;
}
@@ -286,47 +343,54 @@ static void find_non_local_tags(const struct ref *refs,
* fetch.
*/
if (item &&
- !has_sha1_file_with_flags(item->util, OBJECT_INFO_QUICK) &&
- !will_fetch(head, item->util))
- item->util = NULL;
+ !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+ !will_fetch(head, item->oid.hash))
+ oidclr(&item->oid);
item = NULL;
/* skip duplicates and refs that we already have */
- if (string_list_has_string(&remote_refs, ref->name) ||
- string_list_has_string(&existing_refs, ref->name))
+ if (refname_hash_exists(&remote_refs, ref->name) ||
+ refname_hash_exists(&existing_refs, ref->name))
continue;
- item = string_list_insert(&remote_refs, ref->name);
- item->util = (void *)&ref->old_oid;
+ item = refname_hash_add(&remote_refs, ref->name, &ref->old_oid);
+ string_list_insert(&remote_refs_list, ref->name);
}
- string_list_clear(&existing_refs, 1);
+ hashmap_free(&existing_refs, 1);
/*
* We may have a final lightweight tag that needs to be
* checked to see if it needs fetching.
*/
if (item &&
- !has_sha1_file_with_flags(item->util, OBJECT_INFO_QUICK) &&
- !will_fetch(head, item->util))
- item->util = NULL;
+ !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+ !will_fetch(head, item->oid.hash))
+ oidclr(&item->oid);
/*
- * For all the tags in the remote_refs string list,
+ * For all the tags in the remote_refs_list,
* add them to the list of refs to be fetched
*/
- for_each_string_list_item(item, &remote_refs) {
+ for_each_string_list_item(remote_ref_item, &remote_refs_list) {
+ const char *refname = remote_ref_item->string;
+ struct hashmap_entry key;
+
+ hashmap_entry_init(&key, memhash(refname, strlen(refname)));
+ item = hashmap_get(&remote_refs, &key, refname);
+ if (!item)
+ continue; /* can this happen??? */
+
/* Unless we have already decided to ignore this item... */
- if (item->util)
- {
- struct ref *rm = alloc_ref(item->string);
- rm->peer_ref = alloc_ref(item->string);
- oidcpy(&rm->old_oid, item->util);
+ if (!is_null_oid(&item->oid)) {
+ struct ref *rm = alloc_ref(item->refname);
+ rm->peer_ref = alloc_ref(item->refname);
+ oidcpy(&rm->old_oid, &item->oid);
**tail = rm;
*tail = &rm->next;
}
}
-
+ hashmap_free(&remote_refs, 1);
string_list_clear(&remote_refs, 0);
}
--
2.19.0-271-gfe8321ec05
