Re: [PATCH 1/2] fetch-pack: use commit-graph when computing cutoff

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Mon, Jan 31, 2022 at 05:53:14PM -0500, Taylor Blau wrote:
> (+cc Stolee, in case anything I'm saying here is wrong)
> 
> On Fri, Jan 28, 2022 at 11:17:03AM +0100, Patrick Steinhardt wrote:
> > One thing to keep in mind though is that the commit-graph corrects
> > committer dates:
> >
> >     * A commit with at least one parent has corrected committer date
> >       equal to the maximum of its commiter date and one more than the
> >       largest corrected committer date among its parents.
> 
> This snippet refers to how correct committer dates are computed, not how
> the commit dates themselves are stored.
> 
> Indeed, the corrected committer date is used to compute the corrected
> commit date offset, which is the "v2" generation number scheme (as
> opposed to topological levels, which make up "v1").
> 
> But that is entirely separate from the committer dates stored by the
> commit-graph file, which are faithful representations of the exact
> committer date attached to each commit.
> 
> Looking at the very last few lines of the main loop in
> write_graph_chunk_data() (where the committer dates are stored):
> 
>     if (sizeof((*list)->date) > 4)
>       packedDate[0] = htonl(((*list)->date >> 32) & 0x3);
>     else
>       packedDate[0] = 0;
> 
>     packedDate[0] |= htonl(*topo_level_slab_at(ctx->topo_levels, *list) << 2);
>     packedDate[1] = htonl((*list)->date);
>     hashwrite(f, packedDate, 8);
> 
> the low-order 34 bits are used to store the commit's `->date` field, and
> the remaining high-order 30 bits are used to store the generation
> number. (You can look in `fill_commit_graph_info()` to see that we only
> use those 34 bits to write back the date field).
> 
> So I think this paragraph (and the ones related to it) about this being
> an approximation and that being OK since this is a heuristic can all go
> away.
> 
> Thanks,
> Taylor

Aha, that makes a lot of sense. Thanks a lot for correcting my
misconception! I'll send a v2 of this patch series with the corrected
commit message.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]