* Re: Git-gui: crashes on OS X when entering combining ("dead") keys
From: Pat Thoyts @ 2012-02-04 23:07 UTC (permalink / raw)
To: Beat Bolli; +Cc: git
In-Reply-To: <4F2B085C.2000508@drbeat.li>
Beat Bolli <ig@drbeat.li> writes:
>Hi
>
>I've just had git-gui crash on me when I tried to enter the ~ (tilde)
>character on my Mac mini under OS X 10.6.8:
>
This doesn't look git-gui specific so you will likely get more results
posting to the comp.lang.tcl newsgroup about this - or there is a
mac-specific tcl/tk list someplace.
Not posessing a Mac I can't look into this at all - but an
NSException -- that is a problem from the Tk to native layer.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 22:19 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <20120204211544.GC3278@burratino>
On Sat, Feb 4, 2012 at 11:15 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> Title: mailmap: add simple name translation test
>
> Thanks. I guess you think I'm stupid. I have no idea how I can
> correct that assumption and help you to actually work with me to make
> the code better. :/
You mean the commit message, you haven't made any comment about the code.
If you want to know why I had to modify those test assertions, you
really need to look at the code. In essence; all of them use the same
repo, and obviously adding a new commit message changes the output of
the commands.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 21:59 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <20120204211351.GB3278@burratino>
On Sat, Feb 4, 2012 at 11:13 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>> On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> Since I didn't receive a copy of the cover letter or patch 1, I don't
>>> know what this is intended to test _for_. Good --- I can more easily
>>> convey the reaction of future readers who do not necessarily know the
>>> context in which the patch was written (and the commit message does
>>> not seem to say).
> [...]
>> Look at the title:
>> add 'git blame -e' tests
>>
>> s/blame/blame -e/
>
> And? After copy/pasting this particular test with that substitution,
> what do we get a test for?
For 'git blame -e'.
> What class of problem is it supposed to catch?
Problems related to 'git blame -e'?
> By the way, "I blindly copy/pasted" does not seem like a very sensible
> excuse for writing meaningless code (such as the "# git blame" comment
> line). Before the code contained one riddle. Afterwards it has two.
Fine, the drop the patch then... Who needs to test 'git blame -e'
anyway, the current situation of having zero tests for it is perfectly
fine.
Or just apply it. Don't let the perfect be the enemy of the good.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Breakage in master?
From: Erik Faye-Lund @ 2012-02-04 21:55 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, msysGit, Ævar Arnfjörð
In-Reply-To: <CABPQNSZfKCTsuusPpHa2djEOeGVN9z5s_Fr+S3EaHiv7Q4Re9w@mail.gmail.com>
On Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
>>
>>> But here's the REALLY puzzling part: If I add a simple, unused
>>> function to diff-lib.c, like this:
>>> [...]
>>> "git status" starts to error out with that same vsnprintf complaint!
>>>
>>> ---8<---
>>> $ git status
>>> # On branch master
>>> # Changes not staged for commit:
>>> # (use "git add <file>..." to update what will be committed)
>>> fatal: BUG: your vsnprintf is broken (returned -1)
>>> ---8<---
>>
>> OK, that's definitely odd.
>>
>> At the moment of the die() in strbuf_vaddf, what does errno say?
>
> If I apply this patch:
> ---8<---
> diff --git a/strbuf.c b/strbuf.c
> index ff0b96b..52dfdd6 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
> *fmt, va_list ap)
> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
> va_end(cp);
> if (len < 0)
> - die("BUG: your vsnprintf is broken (returned %d)", len);
> + die_errno("BUG: your vsnprintf is broken (returned %d)", len);
> if (len > strbuf_avail(sb)) {
> strbuf_grow(sb, len);
> len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> ---8<---
>
> Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
> too large". This goes both for both failure cases I described. I
> assume this means errno=ERANGE.
>
>> vsnprintf should generally never be returning -1 (it should return the
>> number of characters that would have been written). Since you're on
>> Windows, I assume you're using the replacement version in
>> compat/snprintf.c.
>
> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
> the MinGW target. I'm assuming that means MinGW-runtime has a sane
> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
> the problem occurs. And it's still "Result too large".
>
> So I decided to do a bit of stepping, and it seems libintl takes over
> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
> so it can ensure we support reordering the parameters with $1 etc...
> And aparently this vsnprintf implementation calls the system vnsprintf
> if the format string does not contain '$', and it's using _vsnprintf
> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.
>
> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.
>
Indeed, I just got around to testing this, and doing this on top of
gettext seems to fix the problem for me. For the MSVC, a more
elaborate fix is needed, as it doesn't have a sane vsnprintf.
---
diff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c
index b7cdc5d..f55023e 100644
--- a/gettext-runtime/intl/printf.c
+++ b/gettext-runtime/intl/printf.c
@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)
#if HAVE_SNPRINTF
-# if HAVE_DECL__SNPRINTF
+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)
/* Windows. */
# define system_vsnprintf _vsnprintf
# else
^ permalink raw reply related
* Re: Git performance results on a large repository
From: Greg Troxel @ 2012-02-04 21:42 UTC (permalink / raw)
To: Joshua Redstone; +Cc: git@vger.kernel.org
In-Reply-To: <CB5074CF.3AD7A%joshua.redstone@fb.com>
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
Joshua Redstone <joshua.redstone@fb.com> writes:
> The test repo has 4 million commits, linear history and about 1.3 million
> files. The size of the .git directory is about 15GB, and has been
> repacked with 'git repack -a -d -f --max-pack-size=10g --depth=100
> --window=250'. This repack took about 2 days on a beefy machine (I.e.,
> lots of ram and flash). The size of the index file is 191 MB. I can share
> the script that generated it if people are interested - It basically picks
> 2-5 files, modifies a line or two and adds a few lines at the end
> consisting of random dictionary words, occasionally creates a new file,
> commits all the modifications and repeats.
I have a repository with about 500K files, 3.3G checkout, 1.5G .git, and
about 10K commits. (This is a real repository, not a test case.) So
not as many commits by a lot, but the size seems not so far off.
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
Both of these numbers surprise me. I'm using NetBSD, whose stat
implementation isn't as optimized as Linux (you didn't say, but
assuming). On a years-old desktop, git status seems to be about a
minute semi-cold and 5s warm (once I set the vnode cache big over 500K,
vs 350K default for a 2G ram machine).
So on the warm status, I wonder how big your vnode cache is, and if
you've exceeded it, and I don't follow the cold time at all. Probably
some sort of profiling within git status would be illuminating.
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
So without the stat, I wonder what it's doing that takes 30 minutes.
> One way to get there is to do some deep code modifications to git
> internals, to, for example, create some abstractions and interfaces that
> allow plugging in the specialized servers. Another way is to leave git
> internals as they are and develop a layer of wrapper scripts around all
> the git commands that do the necessary interfacing. The wrapper scripts
> seem perhaps easier in the short-term, but may lead to increasing
> divergence from how git behaves natively and also a layer of complexity.
Having hooks for a blame server cache, etc. sounds sensible. Having a
way to call blames sort of like with --since and then keep updating it
(eg. in emacs) to earlier times sounds useful.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-02-04 21:31 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Johannes Sixt, git, Jeff King
In-Reply-To: <20120127094145.GA2611@burratino>
On Fri, 27 Jan 2012 10:41:45 +0100, Jonathan Nieder <jrnieder@gmail.com>
wrote:
> Frans Klaver wrote:
>
>> Just for my understanding: before a command is executed, a pager
>> (less/more or so) is started? We want to avoid starting the pager if
>> we won't be able to execute the command?
>
> See [1] for an example of a recent patch touching the relevant
> code path.
>
> For example: if I run "git --paginate foo", foo is an alias for bar,
> and the "[pager] bar" configuration is set to point to "otherpager",
> then without this safety git launches the default pager in preparation
> for running git-foo, receives ENOENT from execvp("git-foo"), and then
> the pager has already been launched and it is too late to launch
> otherpager instead.
Took me a while to catch your drift, but if I understand correctly, you're
thinking using some of the code to find out if starting the pager is a
good idea or not. If I factor out the part that finds a command in PATH,
there's the helper that with a fair amount of certainty, will predict
whether 'git foo' will fail with ENOENT or not. It would fix a possible
problem that is currently there. Obviously the only case we can catch, is
the command not actually existing. Although it is just one of the cases
ENOENT can be returned for, I think it is the only one git actually cares
about when checking for it.
>> On Fri, Jan 27, 2012 at 9:48 AM, Jonathan Nieder <jrnieder@gmail.com>
>> wrote:
>
>>> I want to like (b), but the downside seems unacceptable.
>>
>> The downside being: having to figure out what execvp is going to do?
>> That would be tantamount to writing your own execvp.
>
> Exactly.
So as it seems, there are a few cases where we can fairly reliably predict
whether a command is or isn't going to be found. Unless I'm mistaken,
dashed externals are never shell built-ins and so we don't have to be able
to check for their existence. Then assuming that silent_exec_failure
really only cares about commands actually not existing, we can be fairly
naive about it. See if we can find it somewhere in PATH and if we can't
bail out. If we can, start the pager and everything execvp then returns
will be regarded a fatal error. In this case it would be a choice between
spawning the wrong pager, or having a quick browse through the file system.
> That's part of why I was really grateful to Hannes for the reminder to
> take a step back for a moment and consider whether it's worth it.
It may be a sensible reminder. I didn't understand that comment as such.
Maybe it's Hannes' style, I don't know.
> Maybe there's another way or a more targetted way to take care of the
> motivational original confusing scenario that leads to execvp errors.
> (By the way, can you remind me which one that was?)
Been thinking about it and I doubt it. To find out whether EACCES is
returned due to a PATH issue, you have to go through all of those PATH
entries. So while you're at it, there's a lot more you can check and most
of those checks are fairly trivial to do.
I think I've worked through all your review comments. I'll address Hannes'
comments, create an RFC series and see where we end up.
Junio, care to be CC'd in that?
Thanks,
Frans
> [1] http://thread.gmane.org/gmane.comp.version-control.git/179635
^ permalink raw reply
* Re: How to change the index for git status?
From: PJ Weisberg @ 2012-02-04 21:24 UTC (permalink / raw)
To: Peng Yu; +Cc: git
In-Reply-To: <CABrM6wngVDotJoe3Yi5cA_n=JWpLa+M35QEeoKqr1tsp5GD3ng@mail.gmail.com>
On Sat, Feb 4, 2012 at 12:08 PM, Peng Yu <pengyu.ut@gmail.com> wrote:
> Hi,
>
> When I run git status, it show the status compared against the branch
> upstream/develop. I want it to compare against origin/develop. I
> checked progit. But I don't see how to change the branch to compare
> to. Could anybody let me know how to do so? Thanks!
>
> ~/dvcs_src/craftyjs1/Crafty/build/api$ git status
> # On branch 2D_doc_dev
> # Your branch is ahead of 'upstream/develop' by 2 commits.
> #
> nothing to commit (working directory clean)
git branch --set-upstream 2D_doc_dev origin/develop
-PJ
Gehm's Corrollary to Clark's Law: Any technology distinguishable from
magic is insufficiently advanced.
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Jonathan Nieder @ 2012-02-04 21:15 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <CAMP44s1ZPQJzHzYj7e4Kj3Cu+qq0Q3uKrwsE=xS7BmmSqd3gSw@mail.gmail.com>
Felipe Contreras wrote:
> Title: mailmap: add simple name translation test
Thanks. I guess you think I'm stupid. I have no idea how I can
correct that assumption and help you to actually work with me to make
the code better. :/
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 21:13 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <CAMP44s3tmiPGgAUakUgoen2aJcsKw4CygtF5f=4x2dxNTrGbGA@mail.gmail.com>
Felipe Contreras wrote:
> On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Since I didn't receive a copy of the cover letter or patch 1, I don't
>> know what this is intended to test _for_. Good --- I can more easily
>> convey the reaction of future readers who do not necessarily know the
>> context in which the patch was written (and the commit message does
>> not seem to say).
[...]
> Look at the title:
> add 'git blame -e' tests
>
> s/blame/blame -e/
And? After copy/pasting this particular test with that substitution,
what do we get a test for? What class of problem is it supposed to
catch? I do not think a sentence or two is too much to ask for.
By the way, "I blindly copy/pasted" does not seem like a very sensible
excuse for writing meaningless code (such as the "# git blame" comment
line). Before the code contained one riddle. Afterwards it has two.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 21:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <20120204201218.GF22928@burratino>
On Sat, Feb 4, 2012 at 10:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Similar comments to the last patch apply here. This time the patch
> is even more mysterious, since it seems to touch a number of test
> assertions, even while I assume not all of them relate to whatever
> this is supposed to check for.
Title: mailmap: add simple name translation test
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 21:04 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <20120204201027.GE22928@burratino>
On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Since I didn't receive a copy of the cover letter or patch 1, I don't
> know what this is intended to test _for_. Good --- I can more easily
> convey the reaction of future readers who do not necessarily know the
> context in which the patch was written (and the commit message does
> not seem to say).
>
> Looking above, I see
>
> - a lone comment "git blame". What is it trying to tell me? I guess
> you copy/pasted it, but is there any purpose to it?
>
> - a test asserting the claim "Blame output (complex mapping)". This
> title is identical to the test before. I have no idea what this is
> about.
Look at the title:
add 'git blame -e' tests
s/blame/blame -e/
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Jakub Narebski @ 2012-02-04 20:59 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s2gLvXXCTTpAV78=DeJA9dSV793+bx=yJmns7vCwegagQ@mail.gmail.com>
On Sat, 4 Feb 2012, Felipe Contreras wrote:
>
> Typo: improved
Yeah, I have noticed this just as I have send it, but because it is in
_cover letter_ rather than in commit message...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Jonathan Nieder @ 2012-02-04 20:12 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <1328385024-6955-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Similar comments to the last patch apply here. This time the patch
is even more mysterious, since it seems to touch a number of test
assertions, even while I assume not all of them relate to whatever
this is supposed to check for.
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 20:10 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <1328385024-6955-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Thanks for writing tests. I think there is room for a few lines of
explanation above.
[...]
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
> test_cmp expect actual.fuzz
> '
>
> +# git blame
> +cat >expect <<\EOF
> +^OBJI (<author@example.com> DATE 1) one
> +OBJID (<some@dude.xx> DATE 2) two
> +OBJID (<other@author.xx> DATE 3) three
> +OBJID (<other@author.xx> DATE 4) four
> +OBJID (<santa.claus@northpole.xx> DATE 5) five
> +OBJID (<santa.claus@northpole.xx> DATE 6) six
> +OBJID (<cto@company.xx> DATE 7) seven
> +EOF
> +test_expect_success 'Blame output (complex mapping)' '
Since I didn't receive a copy of the cover letter or patch 1, I don't
know what this is intended to test _for_. Good --- I can more easily
convey the reaction of future readers who do not necessarily know the
context in which the patch was written (and the commit message does
not seem to say).
Looking above, I see
- a lone comment "git blame". What is it trying to tell me? I guess
you copy/pasted it, but is there any purpose to it?
- a test asserting the claim "Blame output (complex mapping)". This
title is identical to the test before. I have no idea what this is
about.
Puzzled,
Jonathan
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:08 UTC (permalink / raw)
To: git
In-Reply-To: <CABrM6wm2MochqgC0+FByDA-6nmo0XHtZy1FDXJRkc30+2eyiTw@mail.gmail.com>
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Felipe Contreras @ 2012-02-04 20:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <1328359648-29511-1-git-send-email-jnareb@gmail.com>
On Sat, Feb 4, 2012 at 2:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Typo: improved
--
Felipe Contreras
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:06 UTC (permalink / raw)
To: git
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* RE: Git performance results on a large repository
From: Joshua Redstone @ 2012-02-04 20:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
One more follow-on thought. I imagine that most consumers of git are nowhere near the scale of the test repo that I described. They may still enjoy benefit from efforts to improve git support for large repos. A few possible reasons:
1. The performance improvements should speed things up for smaller repos as well.
2. They may find their repos growing to a 'large scale' at some point in the future.
3. Any code cleanup as part of an effort to support git scalability is good for code base health and e.g., would facilitate future modifications that may more directly affect them.
Cheers,
Josh
________________________________________
From: Nguyen Thai Ngoc Duy [pclouds@gmail.com]
Sent: Friday, February 03, 2012 10:53 PM
To: Joshua Redstone
Cc: git@vger.kernel.org
Subject: Re: Git performance results on a large repository
On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
>
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
Have you tried "git update-index --assume-unchaged"? That should
reduce mass lstat() and hopefully improve the above numbers. The
interface is not exactly easy-to-use, but if it has significant gain,
then we can try to improve UI.
On the index size issue, ideally we should make minimum writes to
index instead of rewriting 191 MB index. An improvement we could do
now is to compress it, reduce disk footprint, thus disk I/O. If you
compress the index with gzip, how big is it?
--
Duy
^ permalink raw reply
* [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras, Jonathan Nieder,
Marius Storm-Olsen
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f6..b1bc521 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
test_cmp expect actual.fuzz
'
+# git blame
+cat >expect <<\EOF
+^OBJI (<author@example.com> DATE 1) one
+OBJID (<some@dude.xx> DATE 2) two
+OBJID (<other@author.xx> DATE 3) three
+OBJID (<other@author.xx> DATE 4) four
+OBJID (<santa.claus@northpole.xx> DATE 5) five
+OBJID (<santa.claus@northpole.xx> DATE 6) six
+OBJID (<cto@company.xx> DATE 7) seven
+EOF
+test_expect_success 'Blame output (complex mapping)' '
+ git blame -e one >actual &&
+ cp actual /tmp &&
+ cp internal_mailmap/.mailmap /tmp &&
+ fuzz_blame actual >actual.fuzz &&
+ test_cmp expect actual.fuzz
+'
+
test_done
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Marius Storm-Olsen,
Jim Meyering, Jonathan Nieder
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index b1bc521..589e39e 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -157,6 +157,9 @@ A U Thor <author@example.com> (1):
CTO <cto@company.xx> (1):
seventh
+Mr. Right <right@company.xx> (1):
+ eight
+
Other Author <other@author.xx> (2):
third
fourth
@@ -196,6 +199,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
test_tick &&
git commit --author "CTO <cto@coompany.xx>" -m seventh &&
+ echo eight >>one &&
+ git add one &&
+ test_tick &&
+ git commit --author "Wrong <right@company.xx>" -m eight &&
+
mkdir -p internal_mailmap &&
echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
@@ -204,6 +212,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
echo "Other Author <other@author.xx> <nick2@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
+ echo "Mr. Right <right@company.xx>" >> internal_mailmap/.mailmap &&
git shortlog -e HEAD >actual &&
test_cmp expect actual
@@ -212,6 +221,9 @@ test_expect_success 'Shortlog output (complex mapping)' '
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
+Author Wrong <right@company.xx> maps to Mr. Right <right@company.xx>
+Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
+
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
@@ -248,6 +260,7 @@ OBJID (Other Author DATE 4) four
OBJID (Santa Claus DATE 5) five
OBJID (Santa Claus DATE 6) six
OBJID (CTO DATE 7) seven
+OBJID (Mr. Right DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame one >actual &&
@@ -264,6 +277,7 @@ OBJID (<other@author.xx> DATE 4) four
OBJID (<santa.claus@northpole.xx> DATE 5) five
OBJID (<santa.claus@northpole.xx> DATE 6) six
OBJID (<cto@company.xx> DATE 7) seven
+OBJID (<right@company.xx> DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame -e one >actual &&
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 1/3] blame: fix email output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Brian Gianforcaro,
Marius Storm-Olsen, Junio C Hamano
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
An extra '>' is added in some cases (<spearce@spearce.org>>), for
example:
% git blame -e -L 947,+7 contrib/completion/git-completion.bash v1.7.9
The current code assumes map_user() *always* returns plain emails, but
that's not true; when there's no email substitution (only name),
map_user() would return 1, but don't touch the mail.
Also, add some tests.
This got broken by d20d654[1].
[1] Change current mailmap usage to do matching on both name and email
of author/committer.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/blame.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..dd69e51 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1403,10 +1403,13 @@ static void get_ac_line(const char *inbuf, const char *what,
* Now, convert both name and e-mail using mailmap
*/
if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
- /* Add a trailing '>' to email, since map_user returns plain emails
- Note: It already has '<', since we replace from mail+1 */
+ /*
+ * Add a trailing '>' to email, since map_user returns plain
+ * emails when it finds a matching mail.
+ * Note: It already has '<', since we replace from mail + 1
+ */
mailpos = memchr(mail, '\0', mail_len);
- if (mailpos && mailpos-mail < mail_len - 1) {
+ if (mailpos && mailpos-mail < mail_len - 1 && *(mailpos - 1) != '>') {
*mailpos = '>';
*(mailpos+1) = '\0';
}
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 0/3] blame: fix output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
The fix, and the tests.
Felipe Contreras (3):
blame: fix email output with mailmap
t: mailmap: add 'git blame -e' tests
t: mailmap: add simple name translation test
builtin/blame.c | 9 ++++++---
t/t4203-mailmap.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
--
1.7.9.1.g97f7d
^ permalink raw reply
* [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-04 19:45 UTC (permalink / raw)
To: git
Git includes protection against rewriting published history on the
receive side with fast-forward check by default (which can be
overridden) and various receive.deny* configuration variables,
including receive.denyNonFastForwards.
Nevertheless git users requested (among others in Git User's Survey)
more help on creation side, namely preventing rewriting parts of
history which was already made public (or at least warning that one is
about to rewrite published history). The "warn before/when rewriting
published history" answer in "17. Which of the following features would
you like to see implemented in git?" multiple-choice question in latest
Git User's Survey 2011[1] got 24% (1525) responses.
[1]: https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
So people would like for git to warn them about rewriting history before
they attempt a push and it turns out to not fast-forward.
What prompted this email is the fact that Mercurial includes support for
tracking which revisions (changesets) are safe to modify in its 2.1
latest version:
http://lwn.net/Articles/478795/
http://mercurial.selenic.com/wiki/WhatsNew
It does that by tracking so called "phase" of a changeset (revision).
http://mercurial.selenic.com/wiki/Phases
http://mercurial.selenic.com/wiki/PhasesDevel
http://www.logilab.org/blogentry/88203
http://www.logilab.org/blogentry/88219
http://www.logilab.org/blogentry/88259
While we don't have to play catch-up with Mercurial features, I think
something similar to what Mercurial has to warn about rewriting
published history (amend, rebase, perhaps even filter-branch) would
be nice to have. Perhaps even follow UI used by Mercurial, and/or
translating its implementation into git terms.
In Mercurial 2.1 there are three available phases: 'public' for
published commits, 'draft' for local un-published commits and
'secret' for local un-published commits which are not meant to
be published.
The phase of a changeset is always equal to or higher than the phase
of it's descendants, according to the following order:
public < draft < secret
Commits start life as 'draft', and move to 'public' on push.
Mercurial documentation talks about phase of a commit, which might
be a good UI, ut also about commits in 'public' phase being "immutable".
As commits in Git are immutable, and rewriting history is in fact
re-doing commits, this description should probably be changed.
While default "push matching" behavior makes it possible to have
"secret" commits, being able to explicitly mark commits as not for
publishing might be a good idea also for Git.
What do you think about this?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Felipe Contreras @ 2012-02-04 19:30 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120204182611.GA31091@sigill.intra.peff.net>
On Sat, Feb 4, 2012 at 8:26 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
>
>> In any case, the one to blame for the header corruption is git:
>> [...]
>> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>>
>> Notice the mail is wrong.
>
> Ugh. The fault lies in this code:
Yes, I found that as well.
> But that comment is wrong. If there's no email mapping needed, map_user
> will leave the "mail" buffer intact, in which case it will have the
> trailing ">" (because we feed the address with enclosing angle
> brackets). So while map_user tries to accept either "foo@example.com\0"
> and "foo@example.com>", it is up to the contents of the mailmap whether
> you get back something with the closing angle bracket or not. Which is a
> pretty error-prone interface.
>
> You can fix it with this:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 5a67c20..9b886fa 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
> /* Add a trailing '>' to email, since map_user returns plain emails
> Note: It already has '<', since we replace from mail+1 */
> mailpos = memchr(mail, '\0', mail_len);
> - if (mailpos && mailpos-mail < mail_len - 1) {
> + if (mailpos && mailpos-mail < mail_len - 1 &&
> + mailpos > mail && *(mailpos-1) != '>') {
> *mailpos = '>';
> *(mailpos+1) = '\0';
> }
Yes, I already did this. I'm writing tests for this right now, but I
think I found yet another bug...
> but it feels like the fix should go into map_user. I tried a few things,
> like "git log -1 --format=%aE", and couldn't find other code paths with
> this problem. So presumably they are all feeding email addresses without
> the closing ">" (so one option is to just say "map_user needs to get
> NUL-terminated strings).
Perhaps, but I though the idea was to make it efficient. I think the
above fix should be ok.
We should have tests for this though, to make sure it doesn't get
broken again. I'm on that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 0/9] respect binary attribute in grep
From: Pete Wyckoff @ 2012-02-04 19:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Thomas Rast, Conrad Irwin, git,
Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120202081747.GA10271@sigill.intra.peff.net>
I took a look at this series. It's nice. My worry was that the
extra open() of non-existent .gitattributes files in all the
directories would cause performance problems across networked
filesystems like NFS.
My usual (non-public) repository has order:
100k files
10k directories
and no files marked as binary. The grep string is such that it
is disk-bound, and not expected to match in any file (or binary):
"time ~/src/git/bin-wrappers/git grep unfindable-string".
With your change, there are 10k new open() calls looking for
.gitattributes in each directory, all of which return ENOENT.
This turns out to have an insignificant impact on performance due
to the much bigger time sink of stat()-ing all the files.
I think this happens to be true because the gitattributes lookups
run in parallel to all the file stat work, as the main thread
dispatches file work while doing its own gitattributes lookups.
It could be plausible that deep directory structures with few
grep-able files will suffer with this change. For example, many
big binary blobs in deep directory hierarchies, but also some
useful files here and there.
One could argue that with the use of .gitattributes to specify
which blobs should not be searched, this series makes this faster
by not having to to read the binary blobs at all. And I'd be
okay with that.
Just FYI that there may be a performance impact on certain
repositories.
-- Pete
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox