Re: adding new 32-bit on-disk (unsigned) timestamp formats (was: [PATCH v5 02/17] pack-mtimes: support reading .mtimes files)

From: Derrick Stolee <derrickstolee@github.com>
Date: May 25, 2022, 13:30 UTC
Message-ID: <32db3720-e9c8-e192-6278-c55855ce1d3e@github.com>
In-Reply-To: <220525.86o7zmt0l0.gmgdl@evledraar.gmail.com>

On 5/25/2022 5:11 AM, Ævar Arnfjörð Bjarmason wrote:

> I must say that I really don't like this part of the format. Is it
> really necessary to optimize the storage space here in a way that leaves
> open questions about future time_t compatibility, and having to
> introduce the first use of unsigned 32 bit timestamps to git's codebase?

The commit-graph file format uses unsigned 34-bit timestamps (packed
with 30-bit topological levels in the CDAT chunk), so this "not-64-bit
signed timestamps" thing is something we've done before.

Show 5 quoted lines

> Yes, this is its own self-contained format, so we don't *need* time_t
> here, but it's also really handy if we can eventually consistently use
> 64 time_t everywhere and not worry about any compatibility issues, or
> unsigned v.s. signed, or to create our own little ext4-like signed 32
> bit timestamp format.

We can also use a new file format version when it is necessary. We have a lot of time to add that detail without overly complicating the format right now.

Show 6 quoted lines

> If we really are trying to micro-optimize storage space here I'm willing
> to bet that this is still a bad/premature optimization. There's much
> better ways to store this sort of data in a compact way if that's the
> concern. E.g. you'd store a 64 bit "base" timestamp in the header for
> the first entry, and have smaller (signed) "delta" timestamps storing
> offsets from that "base" timestamp.

This is a good idea for a v2 format when that is necessary.

Thanks, -Stolee