git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Andreas Schwab <schwab@linux-m68k.org>,
	maximilian attems <max@stro.at>,
	Tay Ray Chuan <rctay89@gmail.com>,
	git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: bug in name-rev on linux-2.6 repo?
Date: Thu, 22 Apr 2010 11:25:04 -0500	[thread overview]
Message-ID: <20100422162504.GA4913@progeny.tock> (raw)
In-Reply-To: <20100422151708.GA15039@coredump.intra.peff.net>

Hi Ted,

maximilian attems attems noticed that ‘git name-rev’ has trouble with
some commits from the ext4 tree [1].  Jeff King investigated:

Jeff King wrote:
> On Thu, Apr 22, 2010 at 10:03:25AM -0500, Jonathan Nieder wrote:
>> Jeff King wrote:

>>> Still looking, but definitely some kind of skew problem.
>>
>> That explains it, then:
>>
>> $ git log --format=%cd' %h' 19f5fb7 ^v2.6.34-rc1~200
>> Sun Jan 24 14:34:07 2010 -0500 19f5fb7
>> Mon Dec 7 10:36:20 2009 -0500 d2eecb0
[...]
> Thanks for confirming, that was the same stretch of history I ended up
> looking at.

It seems that the committer date is set to coincide with the author
date for ext4 patches, which breaks some assumptions by git that each
commit has a later or equal committer date than all parents (modulo
some skew).

How is the ext4 tree generated from your patch queue?

Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/145449

>> Is the rule that every commit must be at most one day before each of
>> its parents?  This should probably be documented somewhere, since it
>> is possible to override the committer date with GIT_COMMITTER_DATE.
>
> There is no hard and fast rule. We have to deal with _some_ clock skew,
> but I think it has been anybody's guess how much. One can always treat
> the graph purely topologically (which is what my first patch removing
> the cutoff_date check did), but that usually means more computation. In
> this case, we go all the way to the roots instead of looking at a
> "recent" subgraph. I think we also look at timestamps in rev-list when
> linearizing to avoid doing a full topo-sort, but I don't remember what
> effects clock skew can have there.
>
> So what should we do with this incident?
>
>   1. Declare it too much clock skew and ignore it.
>
>   2. Drop the cutoff optimization in favor of correctness. We already do
>      this for --stdin, as there is no sensible cutoff for multiple
>      inputs. So you can see how much slower it is:
>
>        $ time git name-rev a1de02dccf906faba2ee2d99cac56799bda3b96a
>        a1de02dccf906faba2ee2d99cac56799bda3b96a undefined
>
>        real    0m0.163s
>        user    0m0.140s
>        sys     0m0.020s
>
>        $ time echo a1de02dccf906faba2ee2d99cac56799bda3b96a |
>          git name-rev --stdin
>        a1de02dccf906faba2ee2d99cac56799bda3b96a (tags/v2.6.34-rc1~199^2~35)
>
>        real    0m3.411s
>        user    0m3.244s
>        sys     0m0.164s
>
>      So perhaps it is something one would want to enable with a
>      command-line option. Or even something we could fall back on
>      automatically as a "slow case" when coming up with an un-nameable
>      rev.
>
>   3. Bump the slop date. 60 days would work here. What's reasonable? A
>      year? At one year, we are still noticeably slower:
>
>        # patched for CUTOFF_SLOP_DATE (365*86400)
>        $ time git name-rev a1de02dccf906faba2ee2d99cac56799bda3b96a
>        a1de02dccf906faba2ee2d99cac56799bda3b96a
>        tags/v2.6.34-rc1~199^2~35
>
>        real    0m1.075s
>        user    0m1.028s
>        sys     0m0.044s
>
> -Peff

  reply	other threads:[~2010-04-22 16:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-21 19:58 bug in name-rev on linux-2.6 repo? maximilian attems
2010-04-22 10:54 ` Tay Ray Chuan
2010-04-22 12:14   ` maximilian attems
2010-04-22 12:40     ` Jonathan Nieder
2010-04-22 14:29       ` Andreas Schwab
2010-04-22 14:44         ` Jeff King
2010-04-22 14:54           ` Jeff King
2010-04-22 15:03             ` Jonathan Nieder
2010-04-22 15:17               ` Jeff King
2010-04-22 16:25                 ` Jonathan Nieder [this message]
2010-04-22 18:20                   ` Linus Torvalds
2010-04-24 23:04                     ` tytso
2010-04-22 14:51         ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100422162504.GA4913@progeny.tock \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=max@stro.at \
    --cc=peff@peff.net \
    --cc=rctay89@gmail.com \
    --cc=schwab@linux-m68k.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).