Re: [PATCH v3 4/5] merge-ort: shuffle the computation and cleanup of potential collisions
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 1, 2022, 09:16 UTC
- Message-ID
- <220701.86o7y9b2ys.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <d3eac3d0bf6539d81751813d89ec5158dcb57338.1656653000.git.gitgitgadget@gmail.com>
On Fri, Jul 01 2022, Elijah Newren via GitGitGadget wrote:
Show 10 quoted lines
> From: Elijah Newren <newren@gmail.com> > > Run compute_collisions() for renames on both sides of history before > any calls to collect_renames(), and do not free the computed collisions > until after both calls to collect_renames(). This is just a code > reorganization at this point that doesn't make sense on its own, but > will permit us to use the computed collision info from both sides > within each call to collect_renames() in a subsequent commit. > > Signed-off-by: Elijah Newren <newren@gmail.com>
This is much easier to read & review with the prep patches, thanks a lot.
B.t.w. on the "legacy code" comment wrt merge-{ort,recursive}.c : I didn't look in that case, but I've seen that you've copied various older code from merge-recursive.c to merge-ort.c (which makes sense) in the past, but I didn't check the origin in that case. Sorry :)
Show 13 quoted lines
> @@ -3106,6 +3105,7 @@ static int detect_and_process_renames(struct merge_options *opt,
> {
> struct diff_queue_struct combined = { 0 };
> struct rename_info *renames = &opt->priv->renames;
> + struct strmap collisions[3];
> int need_dir_renames, s, i, clean = 1;
> unsigned detection_run = 0;
>
> @@ -3155,12 +3155,22 @@ static int detect_and_process_renames(struct merge_options *opt,
> ALLOC_GROW(combined.queue,
> renames->pairs[1].nr + renames->pairs[2].nr,
> combined.alloc);
> + for (int i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) {The "int i" here will need to be pre-declared earlier, per: 6563706568b (CodingGuidelines: give deadline for "for (int i = 0; ...", 2022-03-30)
I also don't mind us just saying "we've waited enough". Junio?
Show 17 quoted lines
> + int other_side = 3 - i;
> + compute_collisions(&collisions[i],
> + &renames->dir_renames[other_side],
> + &renames->pairs[i]);
> + }
> clean &= collect_renames(opt, &combined, MERGE_SIDE1,
> + collisions,
> &renames->dir_renames[2],
> &renames->dir_renames[1]);
> clean &= collect_renames(opt, &combined, MERGE_SIDE2,
> + collisions,
> &renames->dir_renames[1],
> &renames->dir_renames[2]);
> + for (int i = MERGE_SIDE1; i <= MERGE_SIDE2; i++)
> + free_collisions(&collisions[i]);
> STABLE_QSORT(combined.queue, combined.nr, compare_pairs);
> trace2_region_leave("merge", "directory renames", opt->repo);