fetch: replace string-list used as a look-up table with a hashmap
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). Similarly, get_ref_map() uses another string-list as a look-up table for existing refs. Replace it with a hashmap. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Sep 25, 2018 at 13:25 UTC
e198b3a740409fabe5ba774c5f1255b55fdd21c1
1 file changed
+100
-46
builtin/fetch.c
+100
-46
@@ -223,18 +223,6 @@ static void add_merge_config(struct ref **head,
223
}
224
}
225
226
-static int add_existing(const char *refname, const struct object_id *oid,
227
- int flag, void *cbdata)
228
-{
229
- struct string_list *list = (struct string_list *)cbdata;
230
- struct string_list_item *item = string_list_insert(list, refname);
231
- struct object_id *old_oid = xmalloc(sizeof(*old_oid));
232
-
233
- oidcpy(old_oid, oid);
234
- item->util = old_oid;
235
- return 0;
236
-}
237
-
226
static int will_fetch(struct ref **head, const unsigned char *sha1)
227
{
228
struct ref *rm = *head;
@@ -246,16 +234,72 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
234
return 0;
235
}
236
237
+struct refname_hash_entry {
238
+ struct hashmap_entry ent; /* must be the first member */
239
+ struct object_id oid;
240
+ char refname[FLEX_ARRAY];
241
+};
242
+
243
+static int refname_hash_entry_cmp(const void *hashmap_cmp_fn_data,
244
+ const void *e1_,
245
+ const void *e2_,
246
+ const void *keydata)
247
+{
248
+ const struct refname_hash_entry *e1 = e1_;
249
+ const struct refname_hash_entry *e2 = e2_;
250
+
251
+ return strcmp(e1->refname, keydata ? keydata : e2->refname);
252
+}
253
+
254
+static struct refname_hash_entry *refname_hash_add(struct hashmap *map,
255
+ const char *refname,
256
+ const struct object_id *oid)
257
+{
258
+ struct refname_hash_entry *ent;
259
+ size_t len = strlen(refname);
260
+
261
+ FLEX_ALLOC_MEM(ent, refname, refname, len);
262
+ hashmap_entry_init(ent, strhash(refname));
263
+ oidcpy(&ent->oid, oid);
264
+ hashmap_add(map, ent);
265
+ return ent;
266
+}
267
+
268
+static int add_one_refname(const char *refname,
269
+ const struct object_id *oid,
270
+ int flag, void *cbdata)
271
+{
272
+ struct hashmap *refname_map = cbdata;
273
+
274
+ (void) refname_hash_add(refname_map, refname, oid);
275
+ return 0;
276
+}
277
+
278
+static void refname_hash_init(struct hashmap *map)
279
+{
280
+ hashmap_init(map, refname_hash_entry_cmp, NULL, 0);
281
+}
282
+
283
+static int refname_hash_exists(struct hashmap *map, const char *refname)
284
+{
285
+ return !!hashmap_get_from_hash(map, strhash(refname), refname);
286
+}
287
+
288
static void find_non_local_tags(const struct ref *refs,
289
struct ref **head,
290
struct ref ***tail)
291
{
253
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
254
- struct string_list remote_refs = STRING_LIST_INIT_NODUP;
292
+ struct hashmap existing_refs;
293
+ struct hashmap remote_refs;
294
+ struct string_list remote_refs_list = STRING_LIST_INIT_NODUP;
295
+ struct string_list_item *remote_ref_item;
296
const struct ref *ref;
256
- struct string_list_item *item = NULL;
297
+ struct refname_hash_entry *item = NULL;
298
+
299
+ refname_hash_init(&existing_refs);
300
+ refname_hash_init(&remote_refs);
301
258
- for_each_ref(add_existing, &existing_refs);
302
+ for_each_ref(add_one_refname, &existing_refs);
303
for (ref = refs; ref; ref = ref->next) {
304
if (!starts_with(ref->name, "refs/tags/"))
305
continue;
@@ -271,10 +315,10 @@ static void find_non_local_tags(const struct ref *refs,
315
!has_object_file_with_flags(&ref->old_oid,
316
OBJECT_INFO_QUICK) &&
317
!will_fetch(head, ref->old_oid.hash) &&
274
- !has_sha1_file_with_flags(item->util,
318
+ !has_sha1_file_with_flags(item->oid.hash,
319
OBJECT_INFO_QUICK) &&
276
- !will_fetch(head, item->util))
277
- item->util = NULL;
320
+ !will_fetch(head, item->oid.hash))
321
+ oidclr(&item->oid);
322
item = NULL;
323
continue;
324
}
@@ -286,48 +330,53 @@ static void find_non_local_tags(const struct ref *refs,
330
* fetch.
331
*/
332
if (item &&
289
- !has_sha1_file_with_flags(item->util, OBJECT_INFO_QUICK) &&
290
- !will_fetch(head, item->util))
291
- item->util = NULL;
333
+ !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
334
+ !will_fetch(head, item->oid.hash))
335
+ oidclr(&item->oid);
336
337
item = NULL;
338
339
/* skip duplicates and refs that we already have */
296
- if (string_list_has_string(&remote_refs, ref->name) ||
297
- string_list_has_string(&existing_refs, ref->name))
340
+ if (refname_hash_exists(&remote_refs, ref->name) ||
341
+ refname_hash_exists(&existing_refs, ref->name))
342
continue;
343
300
- item = string_list_insert(&remote_refs, ref->name);
301
- item->util = (void *)&ref->old_oid;
344
+ item = refname_hash_add(&remote_refs, ref->name, &ref->old_oid);
345
+ string_list_insert(&remote_refs_list, ref->name);
346
}
303
- string_list_clear(&existing_refs, 1);
347
+ hashmap_free(&existing_refs, 1);
348
349
/*
350
* We may have a final lightweight tag that needs to be
351
* checked to see if it needs fetching.
352
*/
353
if (item &&
310
- !has_sha1_file_with_flags(item->util, OBJECT_INFO_QUICK) &&
311
- !will_fetch(head, item->util))
312
- item->util = NULL;
354
+ !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
355
+ !will_fetch(head, item->oid.hash))
356
+ oidclr(&item->oid);
357
358
/*
315
- * For all the tags in the remote_refs string list,
359
+ * For all the tags in the remote_refs_list,
360
* add them to the list of refs to be fetched
361
*/
318
- for_each_string_list_item(item, &remote_refs) {
362
+ for_each_string_list_item(remote_ref_item, &remote_refs_list) {
363
+ const char *refname = remote_ref_item->string;
364
+
365
+ item = hashmap_get_from_hash(&remote_refs, strhash(refname), refname);
366
+ if (!item)
367
+ BUG("unseen remote ref?");
368
+
369
/* Unless we have already decided to ignore this item... */
320
- if (item->util)
321
- {
322
- struct ref *rm = alloc_ref(item->string);
323
- rm->peer_ref = alloc_ref(item->string);
324
- oidcpy(&rm->old_oid, item->util);
370
+ if (!is_null_oid(&item->oid)) {
371
+ struct ref *rm = alloc_ref(item->refname);
372
+ rm->peer_ref = alloc_ref(item->refname);
373
+ oidcpy(&rm->old_oid, &item->oid);
374
**tail = rm;
375
*tail = &rm->next;
376
}
377
}
329
-
330
- string_list_clear(&remote_refs, 0);
378
+ hashmap_free(&remote_refs, 1);
379
+ string_list_clear(&remote_refs_list, 0);
380
}
381
382
static struct ref *get_ref_map(struct remote *remote,
@@ -343,7 +392,7 @@ static struct ref *get_ref_map(struct remote *remote,
392
/* opportunistically-updated references: */
393
struct ref *orefs = NULL, **oref_tail = &orefs;
394
346
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
395
+ struct hashmap existing_refs;
396
397
if (rs->nr) {
398
struct refspec *fetch_refspec;
@@ -437,19 +486,24 @@ static struct ref *get_ref_map(struct remote *remote,
486
487
ref_map = ref_remove_duplicates(ref_map);
488
440
- for_each_ref(add_existing, &existing_refs);
489
+ refname_hash_init(&existing_refs);
490
+ for_each_ref(add_one_refname, &existing_refs);
491
+
492
for (rm = ref_map; rm; rm = rm->next) {
493
if (rm->peer_ref) {
443
- struct string_list_item *peer_item =
444
- string_list_lookup(&existing_refs,
445
- rm->peer_ref->name);
494
+ const char *refname = rm->peer_ref->name;
495
+ struct refname_hash_entry *peer_item;
496
+
497
+ peer_item = hashmap_get_from_hash(&existing_refs,
498
+ strhash(refname),
499
+ refname);
500
if (peer_item) {
447
- struct object_id *old_oid = peer_item->util;
501
+ struct object_id *old_oid = &peer_item->oid;
502
oidcpy(&rm->peer_ref->old_oid, old_oid);
503
}
504
}
505
}
452
- string_list_clear(&existing_refs, 1);
506
+ hashmap_free(&existing_refs, 1);
507
508
return ref_map;
509
}