| Message ID | cover.1585854639.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Restrict when prefetcing occurs | expand |
Jonathan Tan <jonathantanmy@google.com> writes: > Thanks, everyone, for your review. > > New in v2: > - added restriction on fetching rename_src's blob, following Stolee's > comment > - folded oid_nr==0 check into promisor_remote_get_direct(), following > Stolee's comment > - used "mv server/b server/c", following Stolee's comment > - made diff_add_if_missing() public function, following Junio's comment > > I didn't change the "continue" part that Stolee suggested [1]. > > [1] https://lore.kernel.org/git/xmqqlfng75cl.fsf@gitster.c.googlers.com/ > > Jonathan Tan (2): > promisor-remote: accept 0 as oid_nr in function > diff: restrict when prefetching occurs > > builtin/index-pack.c | 5 ++-- > diff.c | 49 +++++++++++++++++++++++------------ > diffcore-rename.c | 37 +++++++++++++++++++++++++- > diffcore.h | 10 ++++++- > promisor-remote.c | 3 +++ > promisor-remote.h | 8 ++++++ > t/t4067-diff-partial-clone.sh | 48 ++++++++++++++++++++++++++++++++++ > unpack-trees.c | 5 ++-- > 8 files changed, 141 insertions(+), 24 deletions(-) I notice that a439b4ef (diff: skip batch object download when possible, 2020-03-30) by Garima seems to aim for something similar. I'll for now keep both topics with conflict resolution, but it may make sense for you two to compare notes. I especially like the way this series enumerates the formats that matter to prefetching and the way the change is localized in diffcore_std(); the other topic splits a similar logic (with different criteria) between diff_setup_done() and diffcore_std(), which I found suboptimal. Thanks.
On 4/2/2020 4:28 PM, Junio C Hamano wrote: > I notice that a439b4ef (diff: skip batch object download when > possible, 2020-03-30) by Garima seems to aim for something similar. > > I'll for now keep both topics with conflict resolution, but it may > make sense for you two to compare notes. I pointed this out in [1]. I think the right thing to do is for Garima's/my patch to rely on Jonathan's change. The commit needs to be modified, not simply ejected, but it could be separated from the rest of Garima's series. It is only a performance fix for normal clones, but is critical for partial clones. Garima: do you think it would be easy to remove that patch if/when you do a v4 and I can make a new series based on yours and Jonathan's with the rename setting? [1] https://lore.kernel.org/git/b956528c-412b-2f38-bd90-1fa2ae4b8604@gmail.com/ Thanks, -Stolee
On 4/6/2020 7:44 AM, Derrick Stolee wrote: > On 4/2/2020 4:28 PM, Junio C Hamano wrote: >> I notice that a439b4ef (diff: skip batch object download when >> possible, 2020-03-30) by Garima seems to aim for something similar. >> >> I'll for now keep both topics with conflict resolution, but it may >> make sense for you two to compare notes. > > I pointed this out in [1]. I think the right thing to do is for > Garima's/my patch to rely on Jonathan's change. The commit needs > to be modified, not simply ejected, but it could be separated from > the rest of Garima's series. It is only a performance fix for > normal clones, but is critical for partial clones. > > Garima: do you think it would be easy to remove that patch if/when > you do a v4 and I can make a new series based on yours and Jonathan's > with the rename setting? > Sure. I was thinking about rebasing my series on top of Jonathan's and adjusting as necessary, but it might be easier to just remove it and then have a new series based on mine and Jonathan's, like you suggested. There hasn't been any feedback since I sent out v3. I will just re-roll v4 without this patch, to make sure pu no longer requires conflict resolution around the Bloom filter series. Cheers! Garima Singh