On 2/8/2018 4:21 PM, Junio C Hamano wrote:
Derrick Stolee <stolee@xxxxxxxxx> writes:
Add document specifying the binary format for commit graphs. This
format allows for:
* New versions.
* New hash functions and hash lengths.
It still is unclear, at least to me, why OID and OID length are
stored as if they can be independent. If a reader does not
understand a new Object Id hash, is there anything the reader can
still do by knowing how long the hash (which it cannot recompute to
validate) is? And if a reader does know what OID hashing scheme is
used to refer to the objects, it certainly would know how long the
OIDs are.
Giving length may make sense only when a reader can treat these OIDs
as completely opaque identifiers, without having to (re)hash from
the contents, but if that is the case, then there is no point saying
what exact hash function is used to compute OID.
So I'd understand storing only either one or the other, but not
both. Am I missing something?
You're right that this data is redundant. It is easy to describe the
width of the tables using the OID length, so it is convenient to have
that part of the format. Also, it is good to have 4-byte alignment here,
so we are not wasting space.
There isn't a strong reason to put that here, but I don't have a great
reason to remove it, either.
Perhaps leave a byte blank for possible future use?
+The Git commit graph stores a list of commit OIDs and some associated
+metadata, including:
+
+- The generation number of the commit. Commits with no parents have
+ generation number 1; commits with parents have generation number
+ one more than the maximum generation number of its parents. We
+ reserve zero as special, and can be used to mark a generation
+ number invalid or as "not computed".
This "most natural" definition of generation number is stricter than
absolutely necessary (a looser definition that is sufficient is
"gennum of a child is larger than all of its parents'"). While I
personally think that is OK, some people who floated different ideas
in previous discussions on generation numbers may want to articulate
their ideas again. One idea that I found clever was to use the
total number of commits that are ancestors of a commit instead (it
is far more expensive to compute than the most natural gennum, but
doing so may help other topology math, like "describe").
It is more difficult to compute the number of reachable commits, since
you cannot learn that only by looking at the parents (you need to know
how many commits are in the intersection of their reachable sets for a
two-parent merge, or just walk all of the commits). This leads to a
quadratic computation to discover the value for N commits.
I define it this rigidly now because I will submit a patch soon after
this one lands that computes generation numbers and consumes them in
paint_down_to_common(). I've got it sitting in my local repo ready for a
rebase.
+CHUNK LOOKUP:
+
+ (C + 1) * 12 bytes listing the table of contents for the chunks:
+ First 4 bytes describe chunk id. Value 0 is a terminating label.
+ Other 8 bytes provide offset in current file for chunk to start.
+ (Chunks are ordered contiguously in the file, so you can infer
+ the length using the next chunk position if necessary.)
Aren't chunks numbered contiguously, starting from #1, thereby
making it unnecessary to store the 4-byte?
How does a reader obtain the length of the last chunk? Ahh, that is
why there are C+1 entries in this table, not just C, so that the
reader knows where to stop while reading the last one. Does that
mean that this table looks like this?
{ 1, offset_1 },
{ 2, offset_2 },
...
{ C, offset_C },
{ 0, offset_C+1 },
where where (offset_N+1 - offset_N) gives the length of chunk #N?
This is correct.
+ The remaining data in the body is described one chunk at a time, and
+ these chunks may be given in any order. Chunks are required unless
+ otherwise specified.
+
+CHUNK DATA:
+
+ OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
+ The ith entry, F[i], stores the number of OIDs with first
+ byte at most i. Thus F[255] stores the total
+ number of commits (N).
+
+ OID Lookup (ID: {'O', 'I', 'D', 'L'}) (N * H bytes)
+ The OIDs for all commits in the graph, sorted in ascending order.
+
+ Commit Data (ID: {'C', 'G', 'E', 'T' }) (N * (H + 16) bytes)
+ * The first H bytes are for the OID of the root tree.
+ * The next 8 bytes are for the int-ids of the first two parents
+ of the ith commit. Stores value 0xffffffff if no parent in that
+ position. If there are more than two parents, the second value
+ has its most-significant bit on and the other bits store an array
+ position into the Large Edge List chunk.
+ * The next 8 bytes store the generation number of the commit and
+ the commit time in seconds since EPOCH. The generation number
+ uses the higher 30 bits of the first 4 bytes, while the commit
+ time uses the 32 bits of the second 4 bytes, along with the lowest
+ 2 bits of the lowest byte, storing the 33rd and 34th bit of the
+ commit time.
+
+ Large Edge List (ID: {'E', 'D', 'G', 'E'})
+ This list of 4-byte values store the second through nth parents for
+ all octopus merges. The second parent value in the commit data stores
+ an array position within this list along with the most-significant bit
+ on. Starting at that array position, iterate through this list of int-ids
+ for the parents until reaching a value with the most-significant bit on.
+ The other bits correspond to the int-id of the last parent. This chunk
+ should always be present, but may be empty.
I am not convinced about the value of these 4-byte section IDs.
They are useless unless you define what should happen when a reader
sees a block of data with an unknown ID; presence of these IDs given
an impression that you allow a reimplementation to reorder the
sections unnecessarily, especially when all of these are required
anyway, making the canonical reader implementation unnecessarily
complex, no?
One reason to have chunks is to be simple: we have a clear table of
contents at the beginning of the file and can immediately navigate to
portions of the file we care about. The chunk order also doesn't matter
for the purpose of the format.
The true value they present is flexibility: We can extend the v1 format
to include extra metadata in a chunk and insert it anywhere in the
format. If there is some extra information that would be beneficial for
graphs (say, a de-duplicated author list) then we could extend the
format to be v1-compatible.
BUT: I do notice now that load_commit_graph_one() will die() if seeing a
chunk id it doesn't recognize, so that should be fixed if we keep this
chunk model. We should be able to read "v1.1" files if they have extra
chunks (but ignore that data).
Thanks,
-Stolee
