{"thread":{"id":"46718","subject":"Strange behavior of git rev-list","startedAt":"2017-09-07T09:20:45Z","lastAt":"2017-09-08T03:47:50Z","messageCount":8,"participants":["Paweł Marczewski","Jeff King","Stefan Beller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"327698","messageId":"CACNsYJ-UeEONZ+mDgg6x5Bi+D3VmS=5eGCYq1gEPHVsMEJGojA@mail.gmail.com","threadId":"46718","inReplyTo":null,"subject":"Strange behavior of git rev-list","fromName":"Paweł Marczewski","fromEmail":"pwmarcz@gmail.com","sentAt":"2017-09-07T09:20:15Z","receivedAt":"2017-09-07T09:20:45Z","isPatch":false,"sender":{"key":"pwmarcz@gmail.com","avatar":null},"body":"Hi,\nI have an interesting case. In my repository, there are two commits,\n'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git\nrev-list two | grep $(giv rev-parse one)'). However, the output of\n'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.\n\nHere is the repository: https://github.com/pwmarcz/git-wtf/\n\nIt seems that the commit dates influence this behavior, because when I\nedit all the dates to be the same, the output of 'git rev-list\ntwo..one' is empty. Pruning seemingly irrelevant parents also makes it\nempty.\n\nI verified the behavior on git versions 2.14.1, 2.11.0, and on the\n'next' branch (2.14.1.586.g1a2e63c10).\n\nPaweł Marczewski\n"},{"id":"327699","messageId":"20170907094718.b6kuzp2uhvkmwcso@sigill.intra.peff.net","threadId":"46718","inReplyTo":"CACNsYJ-UeEONZ+mDgg6x5Bi+D3VmS=5eGCYq1gEPHVsMEJGojA@mail.gmail.com","subject":"Re: Strange behavior of git rev-list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-07T09:47:18Z","receivedAt":"2017-09-07T09:47:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 07, 2017 at 11:20:15AM +0200, Paweł Marczewski wrote:\n\n> I have an interesting case. In my repository, there are two commits,\n> 'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git\n> rev-list two | grep $(giv rev-parse one)'). However, the output of\n> 'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.\n> \n> Here is the repository: https://github.com/pwmarcz/git-wtf/\n> \n> It seems that the commit dates influence this behavior, because when I\n> edit all the dates to be the same, the output of 'git rev-list\n> two..one' is empty. Pruning seemingly irrelevant parents also makes it\n> empty.\n> \n> I verified the behavior on git versions 2.14.1, 2.11.0, and on the\n> 'next' branch (2.14.1.586.g1a2e63c10).\n\nYes, this is known. The commit dates are used as a proxy for graph\nheight (or generation numbers, if you prefer) so that we avoid having to\nwalk all the way down to a merge base before producing any output. But\nit can give the wrong answer in the face of clock skew.\n\nWe walk back five extra commits more than we need to in order to avoid\nsmall runs of skewed commits, but obviously you can have arbitrary-sized\nruns.\n\n-Peff\n"},{"id":"327700","messageId":"CACNsYJ8rRSf5gNtnPS05CnYCYGmoBymbgH7UumuOuBz1jp6RBA@mail.gmail.com","threadId":"46718","inReplyTo":"20170907094718.b6kuzp2uhvkmwcso@sigill.intra.peff.net","subject":"Re: Strange behavior of git rev-list","fromName":"Paweł Marczewski","fromEmail":"pwmarcz@gmail.com","sentAt":"2017-09-07T09:50:25Z","receivedAt":"2017-09-07T09:50:57Z","isPatch":false,"sender":{"key":"pwmarcz@gmail.com","avatar":null},"body":"Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?\n\nOn 7 September 2017 at 11:47, Jeff King <peff@peff.net> wrote:\n> On Thu, Sep 07, 2017 at 11:20:15AM +0200, Paweł Marczewski wrote:\n>\n>> I have an interesting case. In my repository, there are two commits,\n>> 'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git\n>> rev-list two | grep $(giv rev-parse one)'). However, the output of\n>> 'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.\n>>\n>> Here is the repository: https://github.com/pwmarcz/git-wtf/\n>>\n>> It seems that the commit dates influence this behavior, because when I\n>> edit all the dates to be the same, the output of 'git rev-list\n>> two..one' is empty. Pruning seemingly irrelevant parents also makes it\n>> empty.\n>>\n>> I verified the behavior on git versions 2.14.1, 2.11.0, and on the\n>> 'next' branch (2.14.1.586.g1a2e63c10).\n>\n> Yes, this is known. The commit dates are used as a proxy for graph\n> height (or generation numbers, if you prefer) so that we avoid having to\n> walk all the way down to a merge base before producing any output. But\n> it can give the wrong answer in the face of clock skew.\n>\n> We walk back five extra commits more than we need to in order to avoid\n> small runs of skewed commits, but obviously you can have arbitrary-sized\n> runs.\n>\n> -Peff\n"},{"id":"327701","messageId":"20170907101126.u574pr7l5odff6zo@sigill.intra.peff.net","threadId":"46718","inReplyTo":"CACNsYJ8rRSf5gNtnPS05CnYCYGmoBymbgH7UumuOuBz1jp6RBA@mail.gmail.com","subject":"Re: Strange behavior of git rev-list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-07T10:11:26Z","receivedAt":"2017-09-07T10:11:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 07, 2017 at 11:50:25AM +0200, Paweł Marczewski wrote:\n\n> Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?\n\nI don't think there's a way to turn it off for `rev-list`. Merge-base\ncomputations are more careful, so you could determine the correct\nendpoints that way. But when you fed them to `rev-list`, it would hit\nthe same run of skewed commits.\n\nWe've discussed storing true generation numbers in the past, which would\nmake this optimization more robust, as well as allow us to speed up many\nother traversals (e.g., the \"tag --contains\"). It's something I'd like\nto revisit, but it's not at the top of the pile.\n\nIn the meantime, it would probably be possible to write a patch that\nconditionally disables the optimization. But it would mean all of your\nrev-lists run a _lot_ slower.\n\n-Peff\n"},{"id":"327727","messageId":"CAGZ79kaubRDXnetj+EXawnr50LMo4tGObz+-Bd-=KerndLfSaA@mail.gmail.com","threadId":"46718","inReplyTo":"20170907101126.u574pr7l5odff6zo@sigill.intra.peff.net","subject":"Re: Strange behavior of git rev-list","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-07T19:24:02Z","receivedAt":"2017-09-07T19:24:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Sep 7, 2017 at 3:11 AM, Jeff King <peff@peff.net> wrote:\n> On Thu, Sep 07, 2017 at 11:50:25AM +0200, Paweł Marczewski wrote:\n>\n>> Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?\n>\n> I don't think there's a way to turn it off for `rev-list`. Merge-base\n> computations are more careful, so you could determine the correct\n> endpoints that way. But when you fed them to `rev-list`, it would hit\n> the same run of skewed commits.\n>\n> We've discussed storing true generation numbers in the past, which would\n> make this optimization more robust, as well as allow us to speed up many\n> other traversals (e.g., the \"tag --contains\"). It's something I'd like\n> to revisit, but it's not at the top of the pile.\n\n(We just had an office discussion if this is part of the hash transition plan,\ni.e. have a field in the commit object to contain its generation number.\nand as I think the generation numbers would lead to fast and correct\nbehavior unlike the current heuristic which is only fast, I would try\nto make a strong point actual generation numbers inside commit objects)\n\n>\n> In the meantime, it would probably be possible to write a patch that\n> conditionally disables the optimization. But it would mean all of your\n> rev-lists run a _lot_ slower.\n>\n> -Peff\n"},{"id":"327742","messageId":"20170908031705.5zqs7jhamzgda47t@sigill.intra.peff.net","threadId":"46718","inReplyTo":"CAGZ79kaubRDXnetj+EXawnr50LMo4tGObz+-Bd-=KerndLfSaA@mail.gmail.com","subject":"Re: Strange behavior of git rev-list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-08T03:17:05Z","receivedAt":"2017-09-08T03:17:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 07, 2017 at 12:24:02PM -0700, Stefan Beller wrote:\n\n> > We've discussed storing true generation numbers in the past, which would\n> > make this optimization more robust, as well as allow us to speed up many\n> > other traversals (e.g., the \"tag --contains\"). It's something I'd like\n> > to revisit, but it's not at the top of the pile.\n> \n> (We just had an office discussion if this is part of the hash transition plan,\n> i.e. have a field in the commit object to contain its generation number.\n> and as I think the generation numbers would lead to fast and correct\n> behavior unlike the current heuristic which is only fast, I would try\n> to make a strong point actual generation numbers inside commit objects)\n\nI'm still moderately against storing generation numbers inside the\nobjects. They're redundant with the existing parent pointers, which\nmeans it's an opportunity for the two sets of data to disagree. And as\nwe've seen, once errors are cemented in history it's very hard to fix\nthem, because you break any history built on top.\n\nI'm much more in favor of building a local cache of generation numbers\n(either on the fly or during repacks, where we can piggy-back on the\nexisting pack .idx for indexing).\n\n-Peff\n"},{"id":"327744","messageId":"xmqqwp59zwqv.fsf@gitster.mtv.corp.google.com","threadId":"46718","inReplyTo":"20170908031705.5zqs7jhamzgda47t@sigill.intra.peff.net","subject":"Re: Strange behavior of git rev-list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-08T03:37:28Z","receivedAt":"2017-09-08T03:37:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Sep 07, 2017 at 12:24:02PM -0700, Stefan Beller wrote:\n>\n>> > We've discussed storing true generation numbers in the past, which would\n>> > make this optimization more robust, as well as allow us to speed up many\n>> > other traversals (e.g., the \"tag --contains\"). It's something I'd like\n>> > to revisit, but it's not at the top of the pile.\n>> \n>> (We just had an office discussion if this is part of the hash transition plan,\n>> i.e. have a field in the commit object to contain its generation number.\n>> and as I think the generation numbers would lead to fast and correct\n>> behavior unlike the current heuristic which is only fast, I would try\n>> to make a strong point actual generation numbers inside commit objects)\n>\n> I'm still moderately against storing generation numbers inside the\n> objects. They're redundant with the existing parent pointers, which\n> means it's an opportunity for the two sets of data to disagree. And as\n> we've seen, once errors are cemented in history it's very hard to fix\n> them, because you break any history built on top.\n>\n> I'm much more in favor of building a local cache of generation numbers\n> (either on the fly or during repacks, where we can piggy-back on the\n> existing pack .idx for indexing).\n\nI guess our mails crossed.  Yes, objects that are needlessly broken\nonly because they botch computation of derivable values are real\nproblem, as we need to accomodate them forever because histories can\nand will be built on top of them.\n\nOn the other hand, seeing that the world did not stop even with some\nprojects have trees with entries whose mode are written with 0-padding\non the left in the ancient part of their histories, it might not be\nsuch a big deal.  I dunno.\n"},{"id":"327746","messageId":"20170908034739.4op3w4f2ma5s65ku@sigill.intra.peff.net","threadId":"46718","inReplyTo":"xmqqwp59zwqv.fsf@gitster.mtv.corp.google.com","subject":"Re: Strange behavior of git rev-list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-09-08T03:47:40Z","receivedAt":"2017-09-08T03:47:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 08, 2017 at 12:37:28PM +0900, Junio C Hamano wrote:\n\n> > I'm still moderately against storing generation numbers inside the\n> > objects. They're redundant with the existing parent pointers, which\n> > means it's an opportunity for the two sets of data to disagree. And as\n> > we've seen, once errors are cemented in history it's very hard to fix\n> > them, because you break any history built on top.\n> >\n> > I'm much more in favor of building a local cache of generation numbers\n> > (either on the fly or during repacks, where we can piggy-back on the\n> > existing pack .idx for indexing).\n> \n> I guess our mails crossed.  Yes, objects that are needlessly broken\n> only because they botch computation of derivable values are real\n> problem, as we need to accomodate them forever because histories can\n> and will be built on top of them.\n> \n> On the other hand, seeing that the world did not stop even with some\n> projects have trees with entries whose mode are written with 0-padding\n> on the left in the ancient part of their histories, it might not be\n> such a big deal.  I dunno.\n\nTrue, but in counter-point:\n\n  1. Tree problems generally only affect operations on that tree itself.\n     Parent (or generation number) problems hit us any time we walk\n     across that part of history, which may be much more frequent.\n\n  2. There's an open question of what to do with existing commits\n     without generation numbers.\n\n     Per (1), \"git tag --contains\" is _always_ going to want to know the\n     generation number of v1.0. Some problems are \"local\" to their block\n     of history and as the project history marches forward, the bugs are\n     there but you are less likely to make queries that hit them. But\n     considering old tags for reachability will happen forever (and is\n     the _most_ important use of generation numbers, because it lets us\n     throw out that old history immediately).\n\n     So if we assume we can't rewrite those objects, then we end up with\n     some kind of local cache anyway.\n\n  3. I think we should be moving more in the direction of keeping\n     repo-local caches for optimizations. Reachability bitmaps have been\n     a big performance win. I think we should be doing the same with our\n     properties of commits. Not just generation numbers, but making it\n     cheap to access the graph structure without zlib-inflating whole\n     commit objects (i.e., packv4 or something like the \"metapacks\" I\n     proposed a few years ago).\n\n-Peff\n"}]}