From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id F3F181F487 for ; Tue, 31 Mar 2020 18:21:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726974AbgCaSVh (ORCPT ); Tue, 31 Mar 2020 14:21:37 -0400 Received: from pb-smtp21.pobox.com ([173.228.157.53]:54673 "EHLO pb-smtp21.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726194AbgCaSVg (ORCPT ); Tue, 31 Mar 2020 14:21:36 -0400 Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id BE580B6C3B; Tue, 31 Mar 2020 14:21:34 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=cKuRY/yFSR9VKJiyO9VQSYRCa5I=; b=p8EcdS V3BamlloCHh9OcyVfZIH94e0B1TuZJSAocSJPmBks8SU44Nt0N/OtYGzmgWws3xz 1Nh31ThtLM/BVdoHzyoJoCp8axh/ehOY6fKVcHs1kHf5m5Yc8WLaq6xA/W31xdBL VyeoaxVdVHgTeok6A2l4Hp19lJmZoXOF68mtQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; q=dns; s=sasl; b=LNI3RiQqF2G04Uw4/pmDbpSlQ7w5Fl0c 4kYwILqJc1aQPLKQUBWo864+V5+EikH4UjgMT6Mg1oqVb6vIKNMBjFsBFUR08eCT c7OQtVa9ra8DFzNaZ2AcZBS8iN1jq7beLGGRGCZCGKg3/t6zxyMikrhoEgNs55T9 qsuZsB89ah0= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id A2C85B6C3A; Tue, 31 Mar 2020 14:21:34 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.74.119.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id BF6F6B6C37; Tue, 31 Mar 2020 14:21:31 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: Derrick Stolee Cc: Jonathan Tan , git@vger.kernel.org, peff@peff.net Subject: Re: [PATCH] diff: restrict when prefetching occurs References: <20200331165058.53637-1-jonathantanmy@google.com> Date: Tue, 31 Mar 2020 11:21:30 -0700 In-Reply-To: (Derrick Stolee's message of "Tue, 31 Mar 2020 13:48:10 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 723DC072-737C-11EA-B2CA-8D86F504CC47-77302942!pb-smtp21.pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Derrick Stolee writes: >>>> + for (i = 0; i < rename_dst_nr; i++) { >>>> + if (rename_dst[i].pair) >>>> + continue; /* already found exact match */ >>>> + add_if_missing(options->repo, &to_fetch, rename_dst[i].two); >>> >>> Could this be reversed instead to avoid the "continue"? >> >> Hmm...I prefer the "return early" approach, but can change it if others >> prefer to avoid the "continue" here. > > The "return early" approach is great and makes sense unless there is > only one line of code happening in the other case. Not sure if there > is any potential that the non-continue case grows in size or not. > > Doesn't hurt that much to have the "return early" approach, as you > wrote it. Even with just one statement after the continue, in this particular case, the logic seems to flow a bit more naturally. "Let's see each item in this list. ah, this has already been processed so let's move on. otherwise, we may need to do something a bit more." It also saves one indentation level for the logic that matters ;-) >>>> + for (i = 0; i < rename_src_nr; i++) >>>> + add_if_missing(options->repo, &to_fetch, rename_src[i].p->one); >>> >>> Does this not have the equivalent "rename_src[i].pair" logic for exact >>> matches? One source blob can be copied to multiple destination path, with and without modification, but we currently do not detect the case where a destination blob is a concatenation of two source blobs. So we can optimize the destination side ("we are done with it, no need to look---we won't find anything better anyway as we've found the exact copy source") but we cannot do the same optimization on the source side ("yes, this one was copied to path A, but path B may have a copy with slight modification), I would think.