Re: Small trivial annoyance with the nice new builtin "git am"

From: Jeff King <peff@peff.net>
Date: Jul 29, 2016, 00:21 UTC
Message-ID: <20160729002140.GC9646@sigill.intra.peff.net>
In-Reply-To: <CA+55aFxaia7_VkKKF3JiQt76+z5goz3vCpmWi-wTyBH=iaw5ew@mail.gmail.com>

On Thu, Jul 28, 2016 at 04:35:58PM -0700, Linus Torvalds wrote:

Show 15 quoted lines

> Now, it would be lovely if the new builtin git-am really was *so* fast
> that it applies a 100+-patch series in under a second, but no, that's
> not it. It's just that it only looks up the current time once.
> 
> That seems entirely accidental, I think that what happened is that
> "ident_default_date()" just ends up initializing the default date
> string once, and then the date is cached there, because it's now run
> as a single process for the whole series.
> 
> I think I'd rather get the "real" commit dates, even if they show that
> git only does a handful of commits a second rather than hundreds of
> commits..
> 
> Something that just clears git_default_date in between "git am"
> iterations, perhaps?

Yeah, your analysis makes sense and I think clearing the date between patches is a reasonable solution.

I do wonder if you would be happier giving each commit a "fake" monotonically increasing time, so they are correctly ordered by commit date. I think that runs into some bizarre corner cases, though, like adding 100 patches in 5 seconds, and ending up with commit timestamps just slightly in the future (which is fine if you're then quiet, but skews if you then follow-up in the next 95 seconds with another commit). Compared to skew, having chunks of 20 commits with identical time stamps is probably slightly less bad. At least it reflects reality.

-Peff