diff mbox series

[12/17] delta-islands: convert island_marks khash to use oids

Message ID 20190620074131.GL3713@sigill.intra.peff.net (mailing list archive)
State New, archived
Headers show
Series drop non-object_id hashing | expand

Commit Message

Jeff King June 20, 2019, 7:41 a.m. UTC 
All of the users of this map have an actual "struct object_id" rather
than a bare sha1. Let's use the more descriptive type (and get one step
closer to dropping khash_sha1 entirely).

Signed-off-by: Jeff King <peff@peff.net>
---
 delta-islands.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Jonathan Tan June 20, 2019, 5:38 p.m. UTC | #1 
> @@ -105,7 +105,7 @@ int in_same_island(const struct object_id *trg_oid, const struct object_id *src_
>  	 * If we don't have a bitmap for the target, we can delta it
>  	 * against anything -- it's not an important object
>  	 */
> -	trg_pos = kh_get_sha1(island_marks, trg_oid->hash);
> +	trg_pos = kh_get_oid_map(island_marks, *trg_oid);

[snip]

> @@ -154,7 +154,7 @@ static struct island_bitmap *create_or_get_island_marks(struct object *obj)
>  	khiter_t pos;
>  	int hash_ret;
>  
> -	pos = kh_put_sha1(island_marks, obj->oid.hash, &hash_ret);
> +	pos = kh_put_oid_map(island_marks, obj->oid, &hash_ret);

Thanks for doing this cleanup. The entire series (17 patches) look good
to me. The only remotely surprising thing to me was that OIDs are passed
by value on the stack, both for kh_get_oid_map() and kh_put_oid_map(),
but I see that this is how things currently work (and anyway, changing
this is beyond the scope of this patch set).
Jeff King June 20, 2019, 6:29 p.m. UTC | #2 
On Thu, Jun 20, 2019 at 10:38:23AM -0700, Jonathan Tan wrote:

> > @@ -154,7 +154,7 @@ static struct island_bitmap *create_or_get_island_marks(struct object *obj)
> >  	khiter_t pos;
> >  	int hash_ret;
> >  
> > -	pos = kh_put_sha1(island_marks, obj->oid.hash, &hash_ret);
> > +	pos = kh_put_oid_map(island_marks, obj->oid, &hash_ret);
> 
> Thanks for doing this cleanup. The entire series (17 patches) look good
> to me. The only remotely surprising thing to me was that OIDs are passed
> by value on the stack, both for kh_get_oid_map() and kh_put_oid_map(),
> but I see that this is how things currently work (and anyway, changing
> this is beyond the scope of this patch set).

Thanks for reviewing. Yeah, the pass-by-value surprised me too, as it's
been a while since I've had to touch khash. I think it all cancels out
performance-wise because of the inlining, but it might be a fun thing to
experiment with.

-Peff
diff mbox series

Patch

diff --git a/delta-islands.c b/delta-islands.c
index 5f3ab914f5..88d102298c 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -22,7 +22,7 @@ 
 
 KHASH_INIT(str, const char *, void *, 1, kh_str_hash_func, kh_str_hash_equal)
 
-static khash_sha1 *island_marks;
+static kh_oid_map_t *island_marks;
 static unsigned island_counter;
 static unsigned island_counter_core;
 
@@ -105,7 +105,7 @@  int in_same_island(const struct object_id *trg_oid, const struct object_id *src_
 	 * If we don't have a bitmap for the target, we can delta it
 	 * against anything -- it's not an important object
 	 */
-	trg_pos = kh_get_sha1(island_marks, trg_oid->hash);
+	trg_pos = kh_get_oid_map(island_marks, *trg_oid);
 	if (trg_pos >= kh_end(island_marks))
 		return 1;
 
@@ -113,7 +113,7 @@  int in_same_island(const struct object_id *trg_oid, const struct object_id *src_
 	 * if the source (our delta base) doesn't have a bitmap,
 	 * we don't want to base any deltas on it!
 	 */
-	src_pos = kh_get_sha1(island_marks, src_oid->hash);
+	src_pos = kh_get_oid_map(island_marks, *src_oid);
 	if (src_pos >= kh_end(island_marks))
 		return 0;
 
@@ -129,11 +129,11 @@  int island_delta_cmp(const struct object_id *a, const struct object_id *b)
 	if (!island_marks)
 		return 0;
 
-	a_pos = kh_get_sha1(island_marks, a->hash);
+	a_pos = kh_get_oid_map(island_marks, *a);
 	if (a_pos < kh_end(island_marks))
 		a_bitmap = kh_value(island_marks, a_pos);
 
-	b_pos = kh_get_sha1(island_marks, b->hash);
+	b_pos = kh_get_oid_map(island_marks, *b);
 	if (b_pos < kh_end(island_marks))
 		b_bitmap = kh_value(island_marks, b_pos);
 
@@ -154,7 +154,7 @@  static struct island_bitmap *create_or_get_island_marks(struct object *obj)
 	khiter_t pos;
 	int hash_ret;
 
-	pos = kh_put_sha1(island_marks, obj->oid.hash, &hash_ret);
+	pos = kh_put_oid_map(island_marks, obj->oid, &hash_ret);
 	if (hash_ret)
 		kh_value(island_marks, pos) = island_bitmap_new(NULL);
 
@@ -167,7 +167,7 @@  static void set_island_marks(struct object *obj, struct island_bitmap *marks)
 	khiter_t pos;
 	int hash_ret;
 
-	pos = kh_put_sha1(island_marks, obj->oid.hash, &hash_ret);
+	pos = kh_put_oid_map(island_marks, obj->oid, &hash_ret);
 	if (hash_ret) {
 		/*
 		 * We don't have one yet; make a copy-on-write of the
@@ -279,7 +279,7 @@  void resolve_tree_islands(struct repository *r,
 		struct name_entry entry;
 		khiter_t pos;
 
-		pos = kh_get_sha1(island_marks, ent->idx.oid.hash);
+		pos = kh_get_oid_map(island_marks, ent->idx.oid);
 		if (pos >= kh_end(island_marks))
 			continue;
 
@@ -456,7 +456,7 @@  static void deduplicate_islands(struct repository *r)
 
 void load_delta_islands(struct repository *r)
 {
-	island_marks = kh_init_sha1();
+	island_marks = kh_init_oid_map();
 	remote_islands = kh_init_str();
 
 	git_config(island_config_callback, NULL);
@@ -468,7 +468,7 @@  void load_delta_islands(struct repository *r)
 
 void propagate_island_marks(struct commit *commit)
 {
-	khiter_t pos = kh_get_sha1(island_marks, commit->object.oid.hash);
+	khiter_t pos = kh_get_oid_map(island_marks, commit->object.oid);
 
 	if (pos < kh_end(island_marks)) {
 		struct commit_list *p;
@@ -490,7 +490,7 @@  int compute_pack_layers(struct packing_data *to_pack)
 
 	for (i = 0; i < to_pack->nr_objects; ++i) {
 		struct object_entry *entry = &to_pack->objects[i];
-		khiter_t pos = kh_get_sha1(island_marks, entry->idx.oid.hash);
+		khiter_t pos = kh_get_oid_map(island_marks, entry->idx.oid);
 
 		oe_set_layer(to_pack, entry, 1);