@matheusbsbs / matheus-bagliano / commits / af1c90d13e

fetch-pack: use ref adv. to prune "have" sent

In negotiation using protocol v2, fetch-pack sometimes does not make full use of the information obtained in the ref advertisement: specifically, that if the server advertises a commit that the client also has, the client never needs to inform the server that it has the commit's parents, since it can just tell the server that it has the advertised commit and it knows that the server can and will infer the rest. This is because, in do_fetch_pack_v2(), rev_list_insert_ref_oid() is invoked before mark_complete_and_common_ref(). This means that if we have a commit that is both our ref and their ref, it would be enqueued by rev_list_insert_ref_oid() as SEEN, and since it is thus already SEEN, mark_complete_and_common_ref() would not enqueue it. If mark_complete_and_common_ref() were invoked first, as it is in do_fetch_pack() for protocol v0, then mark_complete_and_common_ref() would enqueue it with COMMON_REF | SEEN. The addition of COMMON_REF ensures that its parents are not sent as "have" lines. Change the order in do_fetch_pack_v2() to be consistent with do_fetch_pack(), and to avoid sending unnecessary "have" lines. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jun 14, 2018 at 15:54 UTC af1c90d13eb325b3fb7b6913757882b663caf1c6

2 files changed +36 -3

fetch-pack.c

+3 -3

@@ -1392,9 +1392,6 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1392 for_each_ref(clear_marks, NULL);

1393 marked = 1;

1394

1395 - for_each_ref(rev_list_insert_ref_oid, NULL);

1396 - for_each_cached_alternate(insert_one_alternate_object);

1397 -

1395 /* Filter 'ref' by 'sought' and those that aren't local */

1396 mark_complete_and_common_ref(args, &ref);

1397 filter_refs(args, &ref, sought, nr_sought);

@@ -1402,6 +1399,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1399 state = FETCH_DONE;

1400 else

1401 state = FETCH_SEND_REQUEST;

1402 +

1403 + for_each_ref(rev_list_insert_ref_oid, NULL);

1404 + for_each_cached_alternate(insert_one_alternate_object);

1405 break;

1406 case FETCH_SEND_REQUEST:

1407 if (send_fetch_request(fd[1], args, ref, &common,

t/t5500-fetch-pack.sh

+33

@@ -755,6 +755,39 @@ test_expect_success 'fetching deepen' '

755 )

756 '

757

758 +test_expect_success 'use ref advertisement to prune "have" lines sent' '

759 + rm -rf server client &&

760 + git init server &&

761 + test_commit -C server both_have_1 &&

762 + git -C server tag -d both_have_1 &&

763 + test_commit -C server both_have_2 &&

764 +

765 + git clone server client &&

766 + test_commit -C server server_has &&

767 + test_commit -C client client_has &&

768 +

769 + # In both protocol v0 and v2, ensure that the parent of both_have_2 is

770 + # not sent as a "have" line. The client should know that the server has

771 + # both_have_2, so it only needs to inform the server that it has

772 + # both_have_2, and the server can infer the rest.

773 +

774 + rm -f trace &&

775 + cp -r client clientv0 &&

776 + GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv0 \

777 + fetch origin server_has both_have_2 &&

778 + grep "have $(git -C client rev-parse client_has)" trace &&

779 + grep "have $(git -C client rev-parse both_have_2)" trace &&

780 + ! grep "have $(git -C client rev-parse both_have_2^)" trace &&

781 +

782 + rm -f trace &&

783 + cp -r client clientv2 &&

784 + GIT_TRACE_PACKET="$(pwd)/trace" git -C clientv2 -c protocol.version=2 \

785 + fetch origin server_has both_have_2 &&

786 + grep "have $(git -C client rev-parse client_has)" trace &&

787 + grep "have $(git -C client rev-parse both_have_2)" trace &&

788 + ! grep "have $(git -C client rev-parse both_have_2^)" trace

789 +'

790 +

791 test_expect_success 'filtering by size' '

792 rm -rf server client &&

793 test_create_repo server &&