@matheusbsbs / matheus-bagliano / commits / 3f83fd5e44
Fork 0

pack-objects: read delta base oid into object_id struct

When we're considering reusing an on-disk delta, we get the oid of the base as a pointer to unsigned char bytes of the hash, either into the packfile itself (for REF_DELTA) or into the pack idx (using the revindex to convert the offset into an index entry). Instead, we'd prefer to use a more type-safe object_id as much as possible. We can get the pack idx using nth_packed_object_id() instead. For the packfile bytes, we can copy them out using oidread(). This doesn't even incur an extra copy overall, since the next thing we'd always do with that pointer is pass it to can_reuse_delta(), which needs an object_id anyway (and called oidread() itself). So this patch also converts that function to take the object_id directly. Note that we did previously use NULL as a sentinel value when the object isn't a delta. We could probably get away with using the null oid for this, but instead we'll use an explicit boolean flag, which should make things more obvious for people reading the code later. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 23, 2020 at 23:29 UTC 3f83fd5e44c1f038c8a7033cb77399e9ef4f43a9
1 file changed +18 -17
builtin/pack-objects.c
+18 -17
@@ -1618,23 +1618,17 @@ static void cleanup_preferred_base(void)
1618 * deltify other objects against, in order to avoid
1619 * circular deltas.
1620 */
1621 -static int can_reuse_delta(const unsigned char *base_sha1,
1621 +static int can_reuse_delta(const struct object_id *base_oid,
1622 struct object_entry *delta,
1623 struct object_entry **base_out)
1624 {
1625 struct object_entry *base;
1626 - struct object_id base_oid;
1627 -
1628 - if (!base_sha1)
1629 - return 0;
1630 -
1631 - oidread(&base_oid, base_sha1);
1626
1627 /*
1628 * First see if we're already sending the base (or it's explicitly in
1629 * our "excluded" list).
1630 */
1637 - base = packlist_find(&to_pack, &base_oid);
1631 + base = packlist_find(&to_pack, base_oid);
1632 if (base) {
1633 if (!in_same_island(&delta->idx.oid, &base->idx.oid))
1634 return 0;
@@ -1647,9 +1641,9 @@ static int can_reuse_delta(const unsigned char *base_sha1,
1641 * even if it was buried too deep in history to make it into the
1642 * packing list.
1643 */
1650 - if (thin && bitmap_has_oid_in_uninteresting(bitmap_git, &base_oid)) {
1644 + if (thin && bitmap_has_oid_in_uninteresting(bitmap_git, base_oid)) {
1645 if (use_delta_islands) {
1652 - if (!in_same_island(&delta->idx.oid, &base_oid))
1646 + if (!in_same_island(&delta->idx.oid, base_oid))
1647 return 0;
1648 }
1649 *base_out = NULL;
@@ -1666,7 +1660,8 @@ static void check_object(struct object_entry *entry)
1660 if (IN_PACK(entry)) {
1661 struct packed_git *p = IN_PACK(entry);
1662 struct pack_window *w_curs = NULL;
1669 - const unsigned char *base_ref = NULL;
1663 + int have_base = 0;
1664 + struct object_id base_ref;
1665 struct object_entry *base_entry;
1666 unsigned long used, used_0;
1667 unsigned long avail;
@@ -1707,9 +1702,13 @@ static void check_object(struct object_entry *entry)
1702 unuse_pack(&w_curs);
1703 return;
1704 case OBJ_REF_DELTA:
1710 - if (reuse_delta && !entry->preferred_base)
1711 - base_ref = use_pack(p, &w_curs,
1712 - entry->in_pack_offset + used, NULL);
1705 + if (reuse_delta && !entry->preferred_base) {
1706 + oidread(&base_ref,
1707 + use_pack(p, &w_curs,
1708 + entry->in_pack_offset + used,
1709 + NULL));
1710 + have_base = 1;
1711 + }
1712 entry->in_pack_header_size = used + the_hash_algo->rawsz;
1713 break;
1714 case OBJ_OFS_DELTA:
@@ -1739,13 +1738,15 @@ static void check_object(struct object_entry *entry)
1738 revidx = find_pack_revindex(p, ofs);
1739 if (!revidx)
1740 goto give_up;
1742 - base_ref = nth_packed_object_sha1(p, revidx->nr);
1741 + if (!nth_packed_object_id(&base_ref, p, revidx->nr))
1742 + have_base = 1;
1743 }
1744 entry->in_pack_header_size = used + used_0;
1745 break;
1746 }
1747
1748 - if (can_reuse_delta(base_ref, entry, &base_entry)) {
1748 + if (have_base &&
1749 + can_reuse_delta(&base_ref, entry, &base_entry)) {
1750 oe_set_type(entry, entry->in_pack_type);
1751 SET_SIZE(entry, in_pack_size); /* delta size */
1752 SET_DELTA_SIZE(entry, in_pack_size);
@@ -1755,7 +1756,7 @@ static void check_object(struct object_entry *entry)
1756 entry->delta_sibling_idx = base_entry->delta_child_idx;
1757 SET_DELTA_CHILD(base_entry, entry);
1758 } else {
1758 - SET_DELTA_EXT(entry, base_ref);
1759 + SET_DELTA_EXT(entry, base_ref.hash);
1760 }
1761
1762 unuse_pack(&w_curs);