Git development
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Jeff King <peff@peff.net>
Cc: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>,
	"Jean-Jacques Lafay" <jeanjacques.lafay@gmail.com>,
	msysgit@googlegroups.com, "Git List" <git@vger.kernel.org>,
	"Philip Oakley" <philipoakley@iee.org>
Subject: Re: Re: [PATCH] git tag --contains : avoid stack overflow
Date: Sun, 11 Nov 2012 23:10:04 +0000 (GMT)	[thread overview]
Message-ID: <alpine.DEB.1.00.1211112308220.5793@bonsai2> (raw)
In-Reply-To: <20121111165431.GA25884@sigill.intra.peff.net>

Hi,

On Sun, 11 Nov 2012, Jeff King wrote:

> On Sun, Nov 11, 2012 at 05:46:32PM +0100, René Scharfe wrote:
> 
> > >However, I couldn't reproduce it on Linux : where the windows
> > >implementations crashes at a ~32000 depth (*not* exactly 32768, mind
> > >you), on linux it happily went through 100000 commits. I didn't take
> > >time to look much further, but maybe on my 64 bit Linux VM, the
> > >process can afford to reserve a much bigger address range for the
> > >stack of each thread than the 1Mb given to 32 bit processes on
> > >windows.  Jean-Jacques.
> > 
> > I can reproduce it on Linux (Debian testing amd64) with ulimit -s 1000
> > to reduce the stack size from its default value of 8MB.
> > 
> > After reverting ffc4b8012d9a4f92ef238ff72c0d15e9e1b402ed (tag: speed
> > up --contains calculation) the test passes even with the smaller
> > stack, but it makes "git tag --contains" take thrice the time as
> > before.
> 
> Right, I am not too surprised.  That function replaced the original
> algorithm with a much faster depth-first recursive one. I haven't looked
> closely yet at Jean-Jacques' iterative adaptation, but that direction
> seems like a good fix for now.
> 
> Ultimately, I have some ideas for doing this in a breadth-first way,
> which would make it more naturally iterative. It would involve having N
> bits of storage per commit to check N tags, but it would mean that we
> could get accurate answers in the face of clock skew (like the
> merge-base calculation, it would merely get slower in the face of skew).
> 
> But since I haven't worked on that at all, fixing the depth-first
> algorithm to be iterative makes sense to me.

Have you tried the latest tag-contains branch of
git://github.com/msysgit/git/? It contains a couple of brush-ups and a
re-write of the recursion (which I hope is right, I had only time to work
on it during an unwanted layover at O'Hare). The SHA-1 is
fc4f42787a0dd0022d202627681362081a66ef70.

Ciao,
Johannes

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

  reply	other threads:[~2012-11-11 23:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1352568970-4669-1-git-send-email-jeanjacques.lafay@gmail.com>
2012-11-10 20:00 ` [PATCH] git tag --contains : avoid stack overflow Philip Oakley
2012-11-10 21:13   ` Jean-Jacques Lafay
2012-11-10 21:33     ` Pat Thoyts
2012-11-11 16:46     ` René Scharfe
2012-11-11 16:54       ` Jeff King
2012-11-11 23:10         ` Johannes Schindelin [this message]
2012-11-12 22:27         ` Jean-Jacques Lafay
2012-11-12 23:14           ` Jeff King
2012-11-13  1:16             ` Johannes Schindelin
2012-11-13  3:46               ` [msysGit] " Jeff King
2012-11-13  4:01                 ` Johannes Schindelin
2012-11-13  4:05                   ` Jeff King
2012-11-13  4:52                     ` Johannes Schindelin
2012-11-13  4:51                 ` Junio C Hamano
2012-11-13  6:08                   ` Jeff King

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=alpine.DEB.1.00.1211112308220.5793@bonsai2 \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=jeanjacques.lafay@gmail.com \
    --cc=msysgit@googlegroups.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.org \
    --cc=rene.scharfe@lsrfire.ath.cx \
    /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