Re: [PATCH 1/6 (v2)] revision caching documentation: man page and technical discussion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nick Edelen <sirnot@xxxxxxxxx> writes:

> As you said, I'll answer most of your questions in the revised docs,
> but I have one question about the time field size.  git uses unsigned
> long internally for its commit date, so it'd be limited by the 32bits
> a long is on 32bit archs.  So, wouldn't the commit times effectively
> be a uin32_t?

The current implementations may be either 32 or 64 bit, but at the on-disk
permanent data structure level they are decimal integers of unlimited
size.  As the rev-cache, similarly to the index, is a local "cache" whose
nature is transient (iow we can afford to say "this new version of git
updates the file format for the rev-cache; your existing rev-cache will be
converted automatically"), its data representation can have limitation
tied to the implementation.  But the limitation should be documented.

As to defining the on-disk data format using C structure, I'd strongly
suggest looking at, learning from and mimicking how read-cache.c and
cache.h handles struct ondisk_cache_entry.  I find writing out a structure
with bitfields as-is, like your patch does, quite iffy.

> Other than that there shouldn't be an overflow problem, unless
> someone's merging over 63 branches in one go.

The current Porcelains may not allow it, but at the data structure level
commit can have arbitrary number of parents, so that assumption is already
broken at the design level.  You need an escape hatch there.

"Because such a merge is so rare, walking the rev-cache that contains such
a commit gives a wrong result without any diagnosis to the caller" is an
unacceptable escape hatch.  It is perfectly fine if the escape hatch is
"because it is so rare, rev-cache marks commits involved in such a path,
and walking the path falls back to actually reading the objects, without
any performance benefit from the cache".  It probably is also Ok if the
escape hatch is "because it is beyond the implementation limit, 'add'
command errors out and rev-cache won't kick in for such a history."

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]