reftable/record: introduce function to compare records by key

In some places we need to sort reftable records by their keys to determine their ordering. This is done by first formatting the keys into a `struct strbuf` and then using `strbuf_cmp()` to compare them. This logic is needlessly roundabout and can end up costing quite a bit of CPU cycles, both due to the allocation and formatting logic. Introduce a new `reftable_record_cmp()` function that knows how to compare two records with each other without requiring allocations. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 12, 2024 at 09:32 UTC adb5d2cbe9a0589540ca631fb42e1579e8ee2d60
2 files changed +68 -1
reftable/record.c
+61 -1
@@ -430,7 +430,6 @@ static int reftable_ref_record_is_deletion_void(const void *p)
430 (const struct reftable_ref_record *)p);
431 }
432
433 -
433 static int reftable_ref_record_equal_void(const void *a,
434 const void *b, int hash_size)
435 {
@@ -439,6 +438,13 @@ static int reftable_ref_record_equal_void(const void *a,
438 return reftable_ref_record_equal(ra, rb, hash_size);
439 }
440
441 +static int reftable_ref_record_cmp_void(const void *_a, const void *_b)
442 +{
443 + const struct reftable_ref_record *a = _a;
444 + const struct reftable_ref_record *b = _b;
445 + return strcmp(a->refname, b->refname);
446 +}
447 +
448 static void reftable_ref_record_print_void(const void *rec,
449 int hash_size)
450 {
@@ -455,6 +461,7 @@ static struct reftable_record_vtable reftable_ref_record_vtable = {
461 .release = &reftable_ref_record_release_void,
462 .is_deletion = &reftable_ref_record_is_deletion_void,
463 .equal = &reftable_ref_record_equal_void,
464 + .cmp = &reftable_ref_record_cmp_void,
465 .print = &reftable_ref_record_print_void,
466 };
467
@@ -625,6 +632,25 @@ static int reftable_obj_record_equal_void(const void *a, const void *b, int hash
632 return 1;
633 }
634
635 +static int reftable_obj_record_cmp_void(const void *_a, const void *_b)
636 +{
637 + const struct reftable_obj_record *a = _a;
638 + const struct reftable_obj_record *b = _b;
639 + int cmp;
640 +
641 + cmp = memcmp(a->hash_prefix, b->hash_prefix,
642 + a->hash_prefix_len > b->hash_prefix_len ?
643 + a->hash_prefix_len : b->hash_prefix_len);
644 + if (cmp)
645 + return cmp;
646 +
647 + /*
648 + * When the prefix is the same then the object record that is longer is
649 + * considered to be bigger.
650 + */
651 + return a->hash_prefix_len - b->hash_prefix_len;
652 +}
653 +
654 static struct reftable_record_vtable reftable_obj_record_vtable = {
655 .key = &reftable_obj_record_key,
656 .type = BLOCK_TYPE_OBJ,
@@ -635,6 +661,7 @@ static struct reftable_record_vtable reftable_obj_record_vtable = {
661 .release = &reftable_obj_record_release,
662 .is_deletion = &not_a_deletion,
663 .equal = &reftable_obj_record_equal_void,
664 + .cmp = &reftable_obj_record_cmp_void,
665 .print = &reftable_obj_record_print,
666 };
667
@@ -953,6 +980,22 @@ static int reftable_log_record_equal_void(const void *a,
980 hash_size);
981 }
982
983 +static int reftable_log_record_cmp_void(const void *_a, const void *_b)
984 +{
985 + const struct reftable_log_record *a = _a;
986 + const struct reftable_log_record *b = _b;
987 + int cmp = strcmp(a->refname, b->refname);
988 + if (cmp)
989 + return cmp;
990 +
991 + /*
992 + * Note that the comparison here is reversed. This is because the
993 + * update index is reversed when comparing keys. For reference, see how
994 + * we handle this in reftable_log_record_key()`.
995 + */
996 + return b->update_index - a->update_index;
997 +}
998 +
999 int reftable_log_record_equal(const struct reftable_log_record *a,
1000 const struct reftable_log_record *b, int hash_size)
1001 {
@@ -1002,6 +1045,7 @@ static struct reftable_record_vtable reftable_log_record_vtable = {
1045 .release = &reftable_log_record_release_void,
1046 .is_deletion = &reftable_log_record_is_deletion_void,
1047 .equal = &reftable_log_record_equal_void,
1048 + .cmp = &reftable_log_record_cmp_void,
1049 .print = &reftable_log_record_print_void,
1050 };
1051
@@ -1077,6 +1121,13 @@ static int reftable_index_record_equal(const void *a, const void *b, int hash_si
1121 return ia->offset == ib->offset && !strbuf_cmp(&ia->last_key, &ib->last_key);
1122 }
1123
1124 +static int reftable_index_record_cmp(const void *_a, const void *_b)
1125 +{
1126 + const struct reftable_index_record *a = _a;
1127 + const struct reftable_index_record *b = _b;
1128 + return strbuf_cmp(&a->last_key, &b->last_key);
1129 +}
1130 +
1131 static void reftable_index_record_print(const void *rec, int hash_size)
1132 {
1133 const struct reftable_index_record *idx = rec;
@@ -1094,6 +1145,7 @@ static struct reftable_record_vtable reftable_index_record_vtable = {
1145 .release = &reftable_index_record_release,
1146 .is_deletion = &not_a_deletion,
1147 .equal = &reftable_index_record_equal,
1148 + .cmp = &reftable_index_record_cmp,
1149 .print = &reftable_index_record_print,
1150 };
1151
@@ -1147,6 +1199,14 @@ int reftable_record_is_deletion(struct reftable_record *rec)
1199 reftable_record_data(rec));
1200 }
1201
1202 +int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b)
1203 +{
1204 + if (a->type != b->type)
1205 + BUG("cannot compare reftable records of different type");
1206 + return reftable_record_vtable(a)->cmp(
1207 + reftable_record_data(a), reftable_record_data(b));
1208 +}
1209 +
1210 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size)
1211 {
1212 if (a->type != b->type)
reftable/record.h
+7
@@ -62,6 +62,12 @@ struct reftable_record_vtable {
62 /* Are two records equal? This assumes they have the same type. Returns 0 for non-equal. */
63 int (*equal)(const void *a, const void *b, int hash_size);
64
65 + /*
66 + * Compare keys of two records with each other. The records must have
67 + * the same type.
68 + */
69 + int (*cmp)(const void *a, const void *b);
70 +
71 /* Print on stdout, for debugging. */
72 void (*print)(const void *rec, int hash_size);
73 };
@@ -114,6 +120,7 @@ struct reftable_record {
120 };
121
122 /* see struct record_vtable */
123 +int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b);
124 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size);
125 void reftable_record_print(struct reftable_record *rec, int hash_size);
126 void reftable_record_key(struct reftable_record *rec, struct strbuf *dest);