* [PATCH 2/2] Fix some memory leaks in various places
From: Mike Hommey @ 2007-12-11 21:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1197407997-22945-1-git-send-email-mh@glandium.org>
Signed-off-by: Mike Hommey <mh@glandium.org>
---
builtin-init-db.c | 1 +
http-walker.c | 10 ++++++++++
walker.c | 2 ++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index e1393b8..df61758 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -415,6 +415,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
safe_create_dir(path, 1);
strcpy(path+len, "/info");
safe_create_dir(path, 1);
+ free(path);
if (shared_repository) {
char buf[10];
diff --git a/http-walker.c b/http-walker.c
index 2c37868..1a02f86 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -231,6 +231,8 @@ static void finish_object_request(struct object_request *obj_req)
{
struct stat st;
+ free(obj_req->url);
+
fchmod(obj_req->local, 0444);
close(obj_req->local); obj_req->local = -1;
@@ -897,9 +899,17 @@ static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
static void cleanup(struct walker *walker)
{
struct walker_data *data = walker->data;
+ struct alt_base *prev_altbase, *altbase = data->alt;
+ while (altbase) {
+ free(altbase->base);
+ prev_altbase = altbase;
+ altbase = altbase->next;
+ free(prev_altbase);
+ }
http_cleanup();
curl_slist_free_all(data->no_pragma_header);
+ free(data);
}
struct walker *get_http_walker(const char *url)
diff --git a/walker.c b/walker.c
index 397b80d..7473e90 100644
--- a/walker.c
+++ b/walker.c
@@ -299,6 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
goto unlock_and_fail;
}
free(msg);
+ free(sha1);
return 0;
@@ -306,6 +307,7 @@ unlock_and_fail:
for (i = 0; i < targets; i++)
if (lock[i])
unlock_ref(lock[i]);
+ free(sha1);
return -1;
}
--
1.5.3.7
^ permalink raw reply related
* Re: git annotate runs out of memory
From: Linus Torvalds @ 2007-12-11 21:14 UTC (permalink / raw)
To: Daniel Berlin, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111122400.25032@woody.linux-foundation.org>
On Tue, 11 Dec 2007, Linus Torvalds wrote:
>
> PS. I also do agree that we seem to use an excessive amount of memory
> there. As to whether it's the same issue or not, I'd not go as far as Nico
> and say "yes" yet. But it's interesting.
I think the answer here is that git-annotate is a totally different issue.
The blame machinery keeps around all the blobs it has ever needed to do a
diff, which explains why something like gcc/ChangeLog blows up badly.
Try this trivial patch.
It will cause us to potentially re-generate some blobs much more, but
that's a reasonably cheap operation, and our delta base cache will get the
expensive cases.
It's still not a free operation, but I get
[torvalds@woody gcc]$ /usr/bin/time ~/git/git-blame gcc/ChangeLog > /dev/null
20.68user 1.25system 0:21.94elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+599833minor)pagefaults 0swaps
so it took 22s and I never saw it grow very large either (it grew to 72M
resident, but I don't know how much of that was the mmap of the
pack-file, so that number is pretty meaningless). Valgrind reports that
it used a maximum heap of about 24M, and almost all of that seems to have
been in the delta cache (which is all good).
Linus
----
builtin-blame.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index c158d31..18f9924 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -87,6 +87,14 @@ struct origin {
char path[FLEX_ARRAY];
};
+static void drop_origin_blob(struct origin *o)
+{
+ if (o->file.ptr) {
+ free(o->file.ptr);
+ o->file.ptr = NULL;
+ }
+}
+
/*
* Given an origin, prepare mmfile_t structure to be used by the
* diff machinery
@@ -558,6 +566,8 @@ static struct patch *get_patch(struct origin *parent, struct origin *origin)
if (!file_p.ptr || !file_o.ptr)
return NULL;
patch = compare_buffer(&file_p, &file_o, 0);
+ drop_origin_blob(parent);
+ drop_origin_blob(origin);
num_get_patch++;
return patch;
}
^ permalink raw reply related
* Re: git annotate runs out of memory
From: Daniel Berlin @ 2007-12-11 21:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111146200.25032@woody.linux-foundation.org>
On 12/11/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 11 Dec 2007, Linus Torvalds wrote:
> >
> > We do that. The expense for git is that we don't do the revisions as a
> > single file at all. We'll look through each commit, check whether the
> > "gcc" directory changed, if it did, we'll go into it, and check whether
> > the "ChangeLog" file changed - and if it did, we'll actually diff it
> > against the previous version.
>
> And, btw: the diff is totally different from the xdelta we have, so even
> if we have an already prepared nice xdelta between the two versions, we'll
> end up re-generating the files in full, and then do a diff on the end
> result.
This is what SVN does as well.
>
> Of course, part of that is that git logically *never* works with deltas,
> except in the actual code-paths that generate objects (or generate packs,
> of course). So even if we had used a delta algorithm that would be
> amenable to be turned into a diff directly, it would have been a layering
> violation to actually do that.
Right. SVN has the same problem.
>
> Other systems can sometimes just re-use their deltas to generate the
> diffs and/or blame information. I dunno whether SVN does that. CVS does,
> afaik.
CVS does because it's delta is line based, so it's easy.
You theroetically can generate blame info from SVN/GIT's block deltas,
but you of course, have the problem GIT does, which is that the delta
is not meant to represent the actual changes that occurred, but
instead, the smallest way to reconstruct data x from data y.
This only sometimes has any relation to how the file actually changed
^ permalink raw reply
* Re: [PATCH] Invert numbers and names in the git-shortlog summary mode.
From: Ingo Molnar @ 2007-12-11 21:13 UTC (permalink / raw)
To: Pierre Habouzit, Jeff King, Jakub Narebski, Christian Couder, git,
Junio C Hamano <g
In-Reply-To: <20071211160744.GE15448@artemis.madism.org>
* Pierre Habouzit <madcoder@debian.org> wrote:
> > for example, if i type "git-checkout" in a Linux kernel tree, it
> > just sits there for up to a minute, and "does nothing". That is
> > totally wrong, human-interaction wise. Then after a minute it just
> > returns. What happened? Why? Where? A newbie would then try
> > "git-checkout -v", using the well-established "verbose" flag, but
> > that gives:
> >
> > Usage: /usr/bin/git-checkout [-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]
>
> not anymore:
>
> $ git checkout -v
> error: unknown switch `v'
> usage: git-branch [options] [<branch>] [<paths>...]
>
> -b ... create a new branch started at <branch>
> -l create the new branchs reflog
> --track tells if the new branch should track the remote branch
> -f proceed even if the index or working tree is not HEAD
> -m performa three-way merge on local modifications if needed
> -q, --quiet be quiet
>
> Not all commands are migrated to this new scheme though.
>
> The next git has a _lot_ of things done better wrt UI and such issues.
> Though some backward incompatible changes must be introduced with the
> proper deprecation warnings, so that people can adapt.
hey, cool! Just when i decide to complain about it, after 2 years of
suffering, it's already fixed in the devel branch =;-) I'll post
suggestions once i have tried out the next version. I'm happy that the
git "first impression" that new users are getting (and the many pitfalls
that they can fall into) is being actively reviewed and improved. It's i
think the main barrier for git world dominance :-)
Ingo
^ permalink raw reply
* Re: git annotate runs out of memory
From: Daniel Berlin @ 2007-12-11 21:09 UTC (permalink / raw)
To: Pierre Habouzit, Daniel Berlin, Linus Torvalds, Matthieu Moy, git
In-Reply-To: <20071211194238.GD20644@artemis.madism.org>
On 12/11/07, Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Dec 11, 2007 at 07:24:54PM +0000, Daniel Berlin wrote:
> > On 12/11/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > >
> > >
> > > On Tue, 11 Dec 2007, Matthieu Moy wrote:
> > > >
> > > > I've seen you pointing this kind of examples many times, but is that
> > > > really different from what even SVN does? "svn log drivers/char" will
> > > > also list atomic commits, and give me a filtered view of the global
> > > > log.
> > >
> > > Ok, BK and CVS both got this horribly wrong, which is why I care. Maybe
> > > this is one of the things SVN gets right.
> > >
> > > I seriously doubt it, though. Do you get *history* right, or do you just
> > > get a random list of commits?
> >
> > No, it will get actual history (IE not just things that happen to have
> > that path in the repository)
>
> OTOH svn has the result right, but the way it does that is horrible.
> When you svn log some/path, I think it just (basically) ask svn log for
> each file in that directory, and merge the logs together. This is "easy"
> for svn since it remembers "where this specific file" came from.
What?
We version directories too.
We don't do svn log for each file in the directory when you request a path.
We look at the history of the path, follow renames, etc.
When you change foo/bar/fred.c, we consider it a change to foo/bar and
foo/, and thus, they have new versions.
I'm not sure where you get this crazy notion that we do anything with
files when you ask about directories.
^ permalink raw reply
* Re: [PATCH] builtin-clone: Implement git clone as a builtin command.
From: Daniel Barkalow @ 2007-12-11 20:59 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <20071211195712.GA3865@bitplanet.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 897 bytes --]
On Tue, 11 Dec 2007, Kristian Høgsberg wrote:
> Ok, don't flame me, I know this isn't appropriate at the moment with
> stabilization for 1.5.4 going on, but I just wanted to post a heads up
> on this work to avoid duplicate effort. It's one big patch at this point
> and I haven't even run the test suite yet, but that will change.
Is that why you misspelled Junio's email address? :)
Also as a heads-up, I've got a builtin-checkout that I've got passing all
the tests (plus a few to test stuff I originally hadn't implemented). This
mostly involved correcting the "interesting" states that unpack_trees()
can leave the index in memory when it returns and figuring out how the
merge code works. I can send it off for review and testing to people who
are interested and don't have other things they should be doing instead.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* git blame with valgrind massif
From: Jon Smirl @ 2007-12-11 20:57 UTC (permalink / raw)
To: Git Mailing List
I ran:
valgrind --tool=massif --heap=yes git blame gcc/ChangeLog
it used about 2.25GB
How do you interpret the massif output?
Command: git blame gcc/ChangeLog
== 0 ===========================
Heap allocation functions accounted for 98.2% of measured spacetime
Called from:
34.3% : 0x44EFC2: patch_delta (git-compat-util.h:235)
24.7% : 0x45DD38: cache_or_unpack_entry (git-compat-util.h:235)
13.1% : 0x4909FC: xdl_cha_alloc (xutils.c:113)
12.0% : 0x461277: strbuf_grow (git-compat-util.h:268)
5.9% : 0x40EF5A: cmd_blame (git-compat-util.h:235)
2.6% : 0x48FA5E: xdl_prepare_env (xprepare.c:79)
1.9% : 0x45D775: unpack_compressed_entry (git-compat-util.h:235)
1.5% : 0x452FE5: read_index_from (git-compat-util.h:284)
1.3% : 0x48F672: xdl_prepare_ctx (xprepare.c:162)
and 61 other insignificant places
== 1 ===========================
Context accounted for 34.3% of measured spacetime
0x44EFC2: patch_delta (git-compat-util.h:235)
Called from:
34.3% : 0x45DA03: unpack_entry (sha1_file.c:1578)
---------------------------------
Context accounted for 24.7% of measured spacetime
0x45DD38: cache_or_unpack_entry (git-compat-util.h:235)
Called from:
24.7% : 0x45F175: read_packed_sha1 (sha1_file.c:1815)
---------------------------------
Context accounted for 13.1% of measured spacetime
0x4909FC: xdl_cha_alloc (xutils.c:113)
Called from:
6.6% : 0x48F75B: xdl_prepare_ctx (xprepare.c:192)
4.0% : 0x48F83A: xdl_prepare_ctx (xprepare.c:115)
---------------------------------
Context accounted for 12.0% of measured spacetime
0x461277: strbuf_grow (git-compat-util.h:268)
Called from:
12.0% : 0x4617F4: strbuf_read (strbuf.c:192)
and 2 other insignificant places
---------------------------------
Context accounted for 5.9% of measured spacetime
0x40EF5A: cmd_blame (git-compat-util.h:235)
Called from:
5.9% : 0x40405A: handle_internal_command (git.c:266)
---------------------------------
Context accounted for 2.6% of measured spacetime
0x48FA5E: xdl_prepare_env (xprepare.c:79)
Called from:
2.6% : 0x48F21A: xdl_do_diff (xdiffi.c:332)
---------------------------------
Context accounted for 1.9% of measured spacetime
0x45D775: unpack_compressed_entry (git-compat-util.h:235)
Called from:
1.9% : 0x45D945: unpack_entry (sha1_file.c:1606)
and 1 other insignificant place
---------------------------------
Context accounted for 1.5% of measured spacetime
0x452FE5: read_index_from (git-compat-util.h:284)
Called from:
1.5% : 0x40FAD7: cmd_blame (builtin-blame.c:2075)
---------------------------------
Context accounted for 1.3% of measured spacetime
0x48F672: xdl_prepare_ctx (xprepare.c:162)
Called from:
0.6% : 0x48FAAB: xdl_prepare_env (xprepare.c:280)
and 1 other insignificant place
== 2 ===========================
Context accounted for 34.3% of measured spacetime
0x44EFC2: patch_delta (git-compat-util.h:235)
0x45DA03: unpack_entry (sha1_file.c:1578)
Called from:
41.7% : 0x45D9BE: unpack_entry (sha1_file.c:1567)
33.3% : 0x45F175: read_packed_sha1 (sha1_file.c:1815)
---------------------------------
Context accounted for 24.7% of measured spacetime
0x45DD38: cache_or_unpack_entry (git-compat-util.h:235)
0x45F175: read_packed_sha1 (sha1_file.c:1815)
Called from:
24.7% : 0x45F55D: read_sha1_file (sha1_file.c:1881)
---------------------------------
Context accounted for 6.6% of measured spacetime
0x4909FC: xdl_cha_alloc (xutils.c:113)
0x48F75B: xdl_prepare_ctx (xprepare.c:192)
Called from:
2.9% : 0x48FAAB: xdl_prepare_env (xprepare.c:280)
1.6% : 0x48FAD0: xdl_prepare_env (xprepare.c:285)
---------------------------------
Context accounted for 4.0% of measured spacetime
0x4909FC: xdl_cha_alloc (xutils.c:113)
0x48F83A: xdl_prepare_ctx (xprepare.c:115)
Called from:
4.0% : 0x48FAAB: xdl_prepare_env (xprepare.c:280)
and 1 other insignificant place
---------------------------------
Context accounted for 12.0% of measured spacetime
0x461277: strbuf_grow (git-compat-util.h:268)
0x4617F4: strbuf_read (strbuf.c:192)
Called from:
12.0% : 0x461881: strbuf_read_file (strbuf.c:228)
---------------------------------
Context accounted for 5.9% of measured spacetime
0x40EF5A: cmd_blame (git-compat-util.h:235)
0x40405A: handle_internal_command (git.c:266)
Called from:
5.9% : 0x4045C9: main (git.c:456)
---------------------------------
Context accounted for 2.6% of measured spacetime
0x48FA5E: xdl_prepare_env (xprepare.c:79)
0x48F21A: xdl_do_diff (xdiffi.c:332)
Called from:
2.6% : 0x48F3B0: xdl_diff (xdiffi.c:542)
---------------------------------
Context accounted for 1.9% of measured spacetime
0x45D775: unpack_compressed_entry (git-compat-util.h:235)
0x45D945: unpack_entry (sha1_file.c:1606)
Called from:
2.4% : 0x45F175: read_packed_sha1 (sha1_file.c:1815)
and 1 other insignificant place
---------------------------------
Context accounted for 1.5% of measured spacetime
0x452FE5: read_index_from (git-compat-util.h:284)
0x40FAD7: cmd_blame (builtin-blame.c:2075)
Called from:
1.5% : 0x40405A: handle_internal_command (git.c:266)
---------------------------------
Context accounted for 0.6% of measured spacetime
0x48F672: xdl_prepare_ctx (xprepare.c:162)
0x48FAAB: xdl_prepare_env (xprepare.c:280)
Called from:
0.6% : 0x48F21A: xdl_do_diff (xdiffi.c:332)
=================================
End of information. Rerun with a bigger --depth value for more.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [ANNOUNCE] ugit: a pyqt-based git gui // was: Re: If you would write git from scratch now, what would you change?
From: David @ 2007-12-11 20:54 UTC (permalink / raw)
To: Marco Costalba; +Cc: Jason Sewall, Andy Parkins, git
In-Reply-To: <e5bfff550712111133j66c4b9adx9f57661cc720aa41@mail.gmail.com>
On Dec 11, 2007 11:33 AM, Marco Costalba <mcostalba@gmail.com> wrote:
> On Dec 11, 2007 8:14 PM, Jason Sewall <jasonsewall@gmail.com> wrote:
> > On Dec 11, 2007 1:20 PM, Marco Costalba <mcostalba@gmail.com> wrote:
>
> > ./configure --prefix=/usr fixed that for me (and I'm sure there's a
> > way to tell python to look in /usr/local... too, but I can't be
> > bothered with that.
> >
>
> ./configure --prefix=$HOME/bin
>
> (half) worked thanks.
>
Hello
To allow python to find the libs you just have to set $PYTHONPATH to
include $PREFIX/lib/python2.4/site-packages (change that 2.4 to 2.5
if you're on python2.5). Of course in a packaged form that wouldn't
be an issue since $PREFIX=/usr, but for test-driving it you'd probably
need to set PYTHONPATH. Are there any distros that don't use
$PREFIX/lib/python2.x/site-packages? If not, it wouldn't hurt to add
an assumption about the installation layout into the main script.
> > I re-installed without the prefix and that error disappeared, but now I get
> > Traceback (most recent call last):
> > File "/usr/local/bin/ugit", line 12, in <module>
> > view = GitView (app.activeWindow())
> > File "../py/views.py", line 15, in __init__
> > File "default/ui/Window.py", line 43, in setupUi
> > AttributeError: setLeftMargin
As for the setLeftMarginError -- That could be because you have py/qt
4.2. The ui files were generated with designer-qt4 (4.3.x) so you
might need a more recent pyqt4. I'll see if I can grab an older
version of pyqt and use it for all of the ui designs (.ui files are
probably forward but not backwards compatible).
^ permalink raw reply
* Re: 'git fast-export' is crashing on the gcc repo
From: Marco Costalba @ 2007-12-11 20:35 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.99999.0712111509270.555@xanadu.home>
On Dec 11, 2007 9:27 PM, Nicolas Pitre <nico@cam.org> wrote:
>
> And then running it again produced:
>
> fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device
>
> adding to today's confusion.
>
GCC repo is turning out to be a very good test case indeed ;-)
^ permalink raw reply
* Re: Something is broken in repack
From: Andreas Ericsson @ 2007-12-11 20:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Jon Smirl, Nicolas Pitre, gcc, Git Mailing List
In-Reply-To: <7v7ijldnq1.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> So what you actually want to do is to just re-use already packed delta
>> chains directly, which is what we normally do. But you are explicitly
>> looking at the "--no-reuse-delta" (aka "git repack -f") case, which is why
>> it then blows up.
>
> While that does not explain, as Nico pointed out, the huge difference
> between the two repack runs that have different starting pack, I would
> say it is a fair thing to say. If you have a suboptimal pack (i.e. not
> enough reusable deltas, as in the 2.1GB pack case), do run "repack -f",
> but if you have a good pack (i.e. 300MB pack), don't.
I think this is too much of a mystery for a lot of people to let it go.
Even I started looking into it, and I've got so little spare time just
now that I wouldn't stand much of a chance of making a contribution
even if I had written the code originally.
That being said, I the fact that some git repositories really *can't*
be repacked on some machines (because it eats ALL virtual memory) is
really something that lowers git's reputation among huge projects.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: git annotate runs out of memory
From: Jon Smirl @ 2007-12-11 20:31 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, Daniel Berlin, git
In-Reply-To: <alpine.LFD.0.99999.0712111403080.555@xanadu.home>
On 12/11/07, Nicolas Pitre <nico@cam.org> wrote:
> On Tue, 11 Dec 2007, Linus Torvalds wrote:
>
> > That said, I'll see if I can speed up "git blame" on the gcc repository.
> > It _is_ a fundamentally much more expensive operation than it is for
> > systems that do single-file things.
>
> It has no excuse for eating up to 1.6GB or RAM though. That's plainly
> wrong.
git blame gcc/ChangeLog
It needs 2.25GB of RAM to run without swapping
That is pretty close to the same number the repack needs.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: git annotate runs out of memory
From: Marco Costalba @ 2007-12-11 20:29 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Linus Torvalds, git
In-Reply-To: <4aca3dc20712111109y5d74a292rf29be6308932393c@mail.gmail.com>
On Dec 11, 2007 8:09 PM, Daniel Berlin <dberlin@dberlin.org> wrote:
>
> In GCC history, it is likely you will be able to cut off at least 30%
> of the time if you do this, because files often have changed entirely
> multiple times.
>
This could be useful for a command line tool but for a GUI the top
down approach is a myth IMHO.
In the GUI case what you actually end up doing (because a GUI allows
it) is to start from the latest file version, check the code region
you are interested then when you find the changed lines you _may_ want
to double click and go to see how it was the file before that change
and then perhaps start a new digging.
I found this is my typical workflow with annotation info because I'm
more interested not in what lines have changed but _why_ have changed
and to do this you naturally end up digging in the past (and checking
also the corresponding revisions patch as example in another tab)
In this case the advantage of oldest to newest annotation algorithm is
that you have _already_ annotated all the history so you can walk and
dig back and forth among the different file versions without *any*
additional delay.
Marco
^ permalink raw reply
* 'git fast-export' is crashing on the gcc repo
From: Nicolas Pitre @ 2007-12-11 20:27 UTC (permalink / raw)
To: git
Simply doing something like:
$ git fast-export --all > /dev/null
results in:
fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
Since this is extremely enlightening, I patched it as follows:
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 2136aad..5c7bfe0 100755
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -104,7 +104,8 @@ static void handle_object(const unsigned char *sha1)
printf("blob\nmark :%d\ndata %lu\n", last_idnum, size);
if (fwrite(buf, size, 1, stdout) != 1)
- die ("Could not write blob %s", sha1_to_hex(sha1));
+ die ("Could not write blob %s: %s",
+ sha1_to_hex(sha1), strerror(errno));
printf("\n");
show_progress();
And then running it again produced:
fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device
adding to today's confusion.
Nicolas
^ permalink raw reply related
* Re: Something is broken in repack
From: Andreas Ericsson @ 2007-12-11 20:26 UTC (permalink / raw)
To: Nicolas Pitre
Cc: David Miller, Linus Torvalds, jonsmirl, Junio C Hamano, gcc, git
In-Reply-To: <alpine.LFD.0.99999.0712111237530.555@xanadu.home>
Nicolas Pitre wrote:
> On Tue, 11 Dec 2007, David Miller wrote:
>
>> From: Nicolas Pitre <nico@cam.org>
>> Date: Tue, 11 Dec 2007 12:21:11 -0500 (EST)
>>
>>> BUT. The point is that repacking the gcc repo using "git repack -a -f
>>> --window=250" has a radically different memory usage profile whether you
>>> do the repack on the earlier 2.1GB pack or the later 300MB pack.
>> If you repack on the smaller pack file, git has to expand more stuff
>> internally in order to search the deltas, whereas with the larger pack
>> file I bet git has to less often undelta'ify to get base objects blobs
>> for delta search.
>
> Of course. I came to that conclusion two days ago. And despite being
> pretty familiar with the involved code (I wrote part of it myself) I
> just can't spot anything wrong with it so far.
>
> But somehow the threading code keep distracting people from that issue
> since it gets to do the same work whether or not the source pack is
> densely packed or not.
>
> Nicolas
> (who wish he had access to a much faster machine to investigate this issue)
If it's still an issue next week, we'll have a 16 core (8 dual-core cpu's)
machine with some 32gb of ram in that'll be free for about two days.
You'll have to remind me about it though, as I've got a lot on my mind
these days.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: git annotate runs out of memory
From: Jakub Narebski @ 2007-12-11 20:14 UTC (permalink / raw)
To: Steven Grimm; +Cc: Linus Torvalds, Daniel Berlin, git
In-Reply-To: <74ED838F-4966-42A9-BC8A-906FD0B4B46F@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> On Dec 11, 2007, at 10:40 AM, Linus Torvalds wrote:
> > To git, "git annotate" is just about the *last* thing you ever want
> > to do.
> > It's not a common operation, it's a "last resort" operation. In git,
> > the
> > whole workflow is designed for "git log -p <pathnamepattern>" rather
> > than
> > annotate/blame.
>
> My use of "git blame" is perhaps not typical, but I use it fairly
> often when I'm looking at a part of my company's code base that I'm
> not terribly familiar with. I've found it's the fastest way to figure
> out who to go ask about a particular block of code that I think is
> responsible for a bug, or more commonly, who to ask to review a change
> I'm making.
>
> "git log" is too coarse-grained to be useful for that purpose; it
> usually doesn't tell me which of the 500 revisions to the file I'm
> looking at introduced the actual line of code I want to change.
There is always "pickaxe" search, i.e.
$ git log -p -S'<string>' -- <file or pathspec>
which can be used instead of blame (perhaps with --follow).
And you can limit blame to the interesting region of file, and to
interesting (important) range of revisions.
[about blame cache]
"git gui blame" uses incremental blame; if only it accepted range
(file fragment) limiting, and if "reblame" (blame --reference=<rev>,
blaming incrementally only lines which changed wrt. given revision)
was implemented.
BTW. qgit actually does blame using it's own "multiple files bottom-up
blame" code (it would be nice to have it in core-git if possible,
hint, hint), and does some caching, although I'm not sure if blame
info also. You should try it, I think.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: git annotate runs out of memory
From: Marco Costalba @ 2007-12-11 20:14 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Daniel Berlin, git
In-Reply-To: <Pine.LNX.4.64.0712111428040.5349@iabervon.org>
On Dec 11, 2007 8:46 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Tue, 11 Dec 2007, Daniel Berlin wrote:
>
> > It is stored in an easier format. However, can you not simply provide
> > side-indexes to do the annotation?
> >
> > I guess that own't work in git because you can change history (in
> > other scm's, history is readonly so you could know the results for
> > committed revisions will never change).
>
> History in git is read-only. It's just that git lets you fork and move
> forward with something different. Each commit can never change (and, in
> fact, you'd have to badly break SHA1 to change it), but which commits are
> relevant to the history can change.
>
Well, revisions never change, but history intended as revision's
parent information could and do changes when you use a path delimiter.
So does the graph that is a direct visualization of parent
information.
For a single revision (that modifies say 3 files) you can have at leat
3 different histories and acutally more if you want to visualize also
the history of the directories trees that owns the modified files.
You end up with a quite big number of different histories all showing
your revisions in different ways, according to the path delimiter you
use.
Perhaps the intended meaning of "changing histories" is this, and in
any case is this the reason you cannot (or has no sense to do) "save"
a single file history in git.
Marco
^ permalink raw reply
* git-cvsexportcommit fails for huge commits
From: Markus Klinik @ 2007-12-11 20:04 UTC (permalink / raw)
To: git
Hi,
git-cvsexportcommit fails for huge commits, that is commits with lots of files.
To be exact, the problem arises if the generated 'cvs status' command exceeds
the maximum length for commands.
Here is the error:
Can't exec "cvs": Argument list too long at /home/mkl/bin/git-cvsexportcommit
line 334. Argument list too long 0 at /home/mkl/bin/git-cvsexportcommit line
334. cvs status [snip: long, long list of files] 1792 at
/home/mkl/bin/git-cvsexportcommit line 332.
The complete error message was 334523 characters long before I snipped the
files.
Used version is from public git repository, Tue Dec 11 20:59:01 CET 2007
(bf82a15095ed374496c2e98b6b672aa8c8c4d034).
^ permalink raw reply
* [New PATCH] Move fetch_ref from http-push.c and http-walker.c to http.c
From: Mike Hommey @ 2007-12-11 20:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20071211074931.GB12959@glandium.org>
Call the moved function http_fetch_ref instead.
Also move needs_quote, hex and quote_ref_url altogether.
Both implementations had a slight difference, so it is not only code moving:
- fetch_ref in http-push.c expects the ref to include "refs/" at the
beginning of its name, and gets the base url from a global variable.
- fetch_ref in http-walker.c expects the ref to *not* include "refs/" at
the beginning (quote_ref_url would add it back), and gets the base url
from a struct walker * argument.
So we actually leave a fetch_ref wrapper function in http-walker.c that
calls the new http_fetch_ref with the proper arguments. In http-push.c,
we just change the only 2 callers to use the new function, with "refs/"
removed from the ref argument.
Signed-off-by: Mike Hommey <mh@glandium.org>
---
Only change is the commit message.
http-push.c | 88 ++------------------------------------------------------
http-walker.c | 80 +---------------------------------------------------
http.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
http.h | 2 +
4 files changed, 89 insertions(+), 163 deletions(-)
diff --git a/http-push.c b/http-push.c
index 610ed9c..a4a9d1c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1063,88 +1063,6 @@ static int fetch_indices(void)
return 0;
}
-static inline int needs_quote(int ch)
-{
- if (((ch >= 'A') && (ch <= 'Z'))
- || ((ch >= 'a') && (ch <= 'z'))
- || ((ch >= '0') && (ch <= '9'))
- || (ch == '/')
- || (ch == '-')
- || (ch == '.'))
- return 0;
- return 1;
-}
-
-static inline int hex(int v)
-{
- if (v < 10) return '0' + v;
- else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
- const char *cp;
- char *dp, *qref;
- int len, baselen, ch;
-
- baselen = strlen(base);
- len = baselen + 1;
- for (cp = ref; (ch = *cp) != 0; cp++, len++)
- if (needs_quote(ch))
- len += 2; /* extra two hex plus replacement % */
- qref = xmalloc(len);
- memcpy(qref, base, baselen);
- for (cp = ref, dp = qref + baselen; (ch = *cp) != 0; cp++) {
- if (needs_quote(ch)) {
- *dp++ = '%';
- *dp++ = hex((ch >> 4) & 0xF);
- *dp++ = hex(ch & 0xF);
- }
- else
- *dp++ = ch;
- }
- *dp = 0;
-
- return qref;
-}
-
-int fetch_ref(char *ref, unsigned char *sha1)
-{
- char *url;
- struct strbuf buffer = STRBUF_INIT;
- char *base = remote->url;
- struct active_request_slot *slot;
- struct slot_results results;
- int ret;
-
- url = quote_ref_url(base, ref);
- slot = get_active_slot();
- slot->results = &results;
- curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
- curl_easy_setopt(slot->curl, CURLOPT_URL, url);
- if (start_active_slot(slot)) {
- run_active_slot(slot);
- if (results.curl_result == CURLE_OK) {
- strbuf_rtrim(&buffer);
- if (buffer.len == 40)
- ret = get_sha1_hex(buffer.buf, sha1);
- else
- ret = 1;
- } else {
- ret = error("Couldn't get %s for %s\n%s",
- url, ref, curl_errorstr);
- }
- } else {
- ret = error("Unable to start request");
- }
-
- strbuf_release(&buffer);
- free(url);
- return ret;
-}
-
static void one_remote_object(const char *hex)
{
unsigned char sha1[20];
@@ -1827,7 +1745,8 @@ static void one_remote_ref(char *refname)
struct object *obj;
int len = strlen(refname) + 1;
- if (fetch_ref(refname, remote_sha1) != 0) {
+ if (http_fetch_ref(remote->url, refname + 5 /* "refs/" */,
+ remote_sha1) != 0) {
fprintf(stderr,
"Unable to fetch ref %s from %s\n",
refname, remote->url);
@@ -1959,7 +1878,8 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)
int len;
char *ref_info;
- if (fetch_ref(ls->dentry_name, remote_sha1) != 0) {
+ if (http_fetch_ref(remote->url, ls->dentry_name + 5 /* "refs/" */,
+ remote_sha1) != 0) {
fprintf(stderr,
"Unable to fetch ref %s from %s\n",
ls->dentry_name, remote->url);
diff --git a/http-walker.c b/http-walker.c
index 4e878b3..2c37868 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -888,88 +888,10 @@ static int fetch(struct walker *walker, unsigned char *sha1)
data->alt->base);
}
-static inline int needs_quote(int ch)
-{
- if (((ch >= 'A') && (ch <= 'Z'))
- || ((ch >= 'a') && (ch <= 'z'))
- || ((ch >= '0') && (ch <= '9'))
- || (ch == '/')
- || (ch == '-')
- || (ch == '.'))
- return 0;
- return 1;
-}
-
-static inline int hex(int v)
-{
- if (v < 10) return '0' + v;
- else return 'A' + v - 10;
-}
-
-static char *quote_ref_url(const char *base, const char *ref)
-{
- const char *cp;
- char *dp, *qref;
- int len, baselen, ch;
-
- baselen = strlen(base);
- len = baselen + 7; /* "/refs/" + NUL */
- for (cp = ref; (ch = *cp) != 0; cp++, len++)
- if (needs_quote(ch))
- len += 2; /* extra two hex plus replacement % */
- qref = xmalloc(len);
- memcpy(qref, base, baselen);
- memcpy(qref + baselen, "/refs/", 6);
- for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
- if (needs_quote(ch)) {
- *dp++ = '%';
- *dp++ = hex((ch >> 4) & 0xF);
- *dp++ = hex(ch & 0xF);
- }
- else
- *dp++ = ch;
- }
- *dp = 0;
-
- return qref;
-}
-
static int fetch_ref(struct walker *walker, char *ref, unsigned char *sha1)
{
- char *url;
- struct strbuf buffer = STRBUF_INIT;
struct walker_data *data = walker->data;
- const char *base = data->alt->base;
- struct active_request_slot *slot;
- struct slot_results results;
- int ret;
-
- url = quote_ref_url(base, ref);
- slot = get_active_slot();
- slot->results = &results;
- curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
- curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
- curl_easy_setopt(slot->curl, CURLOPT_URL, url);
- if (start_active_slot(slot)) {
- run_active_slot(slot);
- if (results.curl_result == CURLE_OK) {
- strbuf_rtrim(&buffer);
- if (buffer.len == 40)
- ret = get_sha1_hex(buffer.buf, sha1);
- else
- ret = 1;
- } else {
- ret = error("Couldn't get %s for %s\n%s",
- url, ref, curl_errorstr);
- }
- } else {
- ret = error("Unable to start request");
- }
-
- strbuf_release(&buffer);
- free(url);
- return ret;
+ return http_fetch_ref(data->alt->base, ref, sha1);
}
static void cleanup(struct walker *walker)
diff --git a/http.c b/http.c
index 784b93e..c6de964 100644
--- a/http.c
+++ b/http.c
@@ -552,3 +552,85 @@ void finish_all_active_slots(void)
slot = slot->next;
}
}
+
+static inline int needs_quote(int ch)
+{
+ if (((ch >= 'A') && (ch <= 'Z'))
+ || ((ch >= 'a') && (ch <= 'z'))
+ || ((ch >= '0') && (ch <= '9'))
+ || (ch == '/')
+ || (ch == '-')
+ || (ch == '.'))
+ return 0;
+ return 1;
+}
+
+static inline int hex(int v)
+{
+ if (v < 10) return '0' + v;
+ else return 'A' + v - 10;
+}
+
+static char *quote_ref_url(const char *base, const char *ref)
+{
+ const char *cp;
+ char *dp, *qref;
+ int len, baselen, ch;
+
+ baselen = strlen(base);
+ len = baselen + 7; /* "/refs/" + NUL */
+ for (cp = ref; (ch = *cp) != 0; cp++, len++)
+ if (needs_quote(ch))
+ len += 2; /* extra two hex plus replacement % */
+ qref = xmalloc(len);
+ memcpy(qref, base, baselen);
+ memcpy(qref + baselen, "/refs/", 6);
+ for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
+ if (needs_quote(ch)) {
+ *dp++ = '%';
+ *dp++ = hex((ch >> 4) & 0xF);
+ *dp++ = hex(ch & 0xF);
+ }
+ else
+ *dp++ = ch;
+ }
+ *dp = 0;
+
+ return qref;
+}
+
+int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1)
+{
+ char *url;
+ struct strbuf buffer = STRBUF_INIT;
+ struct active_request_slot *slot;
+ struct slot_results results;
+ int ret;
+
+ url = quote_ref_url(base, ref);
+ slot = get_active_slot();
+ slot->results = &results;
+ curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
+ curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+ if (start_active_slot(slot)) {
+ run_active_slot(slot);
+ if (results.curl_result == CURLE_OK) {
+ strbuf_rtrim(&buffer);
+ if (buffer.len == 40)
+ ret = get_sha1_hex(buffer.buf, sha1);
+ else
+ ret = 1;
+ } else {
+ ret = error("Couldn't get %s for %s\n%s",
+ url, ref, curl_errorstr);
+ }
+ } else {
+ ret = error("Unable to start request");
+ }
+
+ strbuf_release(&buffer);
+ free(url);
+ return ret;
+}
diff --git a/http.h b/http.h
index 87d638b..b709222 100644
--- a/http.h
+++ b/http.h
@@ -96,4 +96,6 @@ static inline int missing__target(int code, int result)
#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
+extern int http_fetch_ref(const char *base, const char *ref, unsigned char *sha1);
+
#endif /* HTTP_H */
--
1.5.3.7.1161.g4a58
^ permalink raw reply related
* Re: git annotate runs out of memory
From: Junio C Hamano @ 2007-12-11 19:59 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Daniel Berlin, Linus Torvalds, git
In-Reply-To: <20071211193407.GC20644@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
>> Looking through thousands of diffs to find the one that happened to
>> your line is also pretty annoying.
>
> If the question you want to answer is "what happened to that line"
> then using git annotate is using a big hammer for no good reason.
>
> git log -S'<put the content of the line here>' -- path/to/file.c
>
> will give you the very same answer, pointing you to the changes that
> added or removed that line directly. It's not a fast command either, but
> it should be less resource hungry than annotate that has to do roughly
> the same for all lines whereas you're interested in one only.
>
> The direct plus here, is that git log output is incremental, so you have
> answers about the first diffs quite quick, which let you examine the
> first answers while the rest is still being computed.
Yes.
> Unlike git annotate, this also allow you to restrict the revisions
> where it searches to a range where you know this happened, which makes
> it almost instantaneous in most cases.
Yes, but blame also takes revision bottoms (obviously you have to start
digging from a single revision so "blame master..next pu" would not
work, but "blame ^foo ^bar baz" would).
> Of course, if the line is ' free(p);\n' then you will probably have
> quite a few false positives,...
You can feed more than a line from -S, and the assumed and recommended
typical use case is to do so.
> Note that it does not justifies the current memory consumption that just
> looks bad and wrong to me,...
Right.
^ permalink raw reply
* Re: git annotate runs out of memory
From: Linus Torvalds @ 2007-12-11 19:50 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111122400.25032@woody.linux-foundation.org>
On Tue, 11 Dec 2007, Linus Torvalds wrote:
>
> We do that. The expense for git is that we don't do the revisions as a
> single file at all. We'll look through each commit, check whether the
> "gcc" directory changed, if it did, we'll go into it, and check whether
> the "ChangeLog" file changed - and if it did, we'll actually diff it
> against the previous version.
And, btw: the diff is totally different from the xdelta we have, so even
if we have an already prepared nice xdelta between the two versions, we'll
end up re-generating the files in full, and then do a diff on the end
result.
Of course, part of that is that git logically *never* works with deltas,
except in the actual code-paths that generate objects (or generate packs,
of course). So even if we had used a delta algorithm that would be
amenable to be turned into a diff directly, it would have been a layering
violation to actually do that.
Other systems can sometimes just re-use their deltas to generate the
diffs and/or blame information. I dunno whether SVN does that. CVS does,
afaik.
Linus
^ permalink raw reply
* Re: [PATCH] Fix a typo in checkout.sh and cleanup one-line help messages
From: Junio C Hamano @ 2007-12-11 19:49 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Pierre Habouzit, Git ML
In-Reply-To: <475EBC90.1060606@viscovery.net>
Thanks. Will take yours.
^ permalink raw reply
* Re: git annotate runs out of memory
From: Daniel Barkalow @ 2007-12-11 19:46 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Marco Costalba, git
In-Reply-To: <4aca3dc20712111103s1af3b045h484ea749378c6282@mail.gmail.com>
On Tue, 11 Dec 2007, Daniel Berlin wrote:
> It is stored in an easier format. However, can you not simply provide
> side-indexes to do the annotation?
>
> I guess that own't work in git because you can change history (in
> other scm's, history is readonly so you could know the results for
> committed revisions will never change).
History in git is read-only. It's just that git lets you fork and move
forward with something different. Each commit can never change (and, in
fact, you'd have to badly break SHA1 to change it), but which commits are
relevant to the history can change.
Keeping extra information is fine; at worst, it'll go irrelevant.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: git annotate runs out of memory
From: Pierre Habouzit @ 2007-12-11 19:42 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Linus Torvalds, Matthieu Moy, git
In-Reply-To: <4aca3dc20712111124y1d9171eem4d2c4f0872703786@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]
On Tue, Dec 11, 2007 at 07:24:54PM +0000, Daniel Berlin wrote:
> On 12/11/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >
> > On Tue, 11 Dec 2007, Matthieu Moy wrote:
> > >
> > > I've seen you pointing this kind of examples many times, but is that
> > > really different from what even SVN does? "svn log drivers/char" will
> > > also list atomic commits, and give me a filtered view of the global
> > > log.
> >
> > Ok, BK and CVS both got this horribly wrong, which is why I care. Maybe
> > this is one of the things SVN gets right.
> >
> > I seriously doubt it, though. Do you get *history* right, or do you just
> > get a random list of commits?
>
> No, it will get actual history (IE not just things that happen to have
> that path in the repository)
OTOH svn has the result right, but the way it does that is horrible.
When you svn log some/path, I think it just (basically) ask svn log for
each file in that directory, and merge the logs together. This is "easy"
for svn since it remembers "where this specific file" came from.
So for svn it's just a matter of merging the individual files histories
together. It may have a more clever implementation, but basically I
believe it would be similar to that in the end.
Of course, if you do something as stupid as:
svn cp Makefile some/path/foo.c
# completely rewrite foo.c
svn commit
then you'll have the history of `Makefile` melded into the
some/path/foo.c svn log, which is completely horribly wrong.
or if you do (which unlike the previous example isn't silly for so
many good reasons):
cp bar.c foo.c
svn add foo.c
svn commit
then foo.c won't have bar.c history in its svn log.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git annotate runs out of memory
From: Linus Torvalds @ 2007-12-11 19:42 UTC (permalink / raw)
To: Daniel Berlin; +Cc: Git Mailing List
In-Reply-To: <4aca3dc20712111109y5d74a292rf29be6308932393c@mail.gmail.com>
On Tue, 11 Dec 2007, Daniel Berlin wrote:
>
> I understand this, and completely agree with you.
> However, I cannot force GCC people to adopt completely new workflow in
> this regard.
Oh, I agree. It's why we do have "git blame" these days, and it's why I've
tried to make people use the nicer incremental mode, which is not at all
faster, but it's a hell of a lot more pleasant to use because you get some
output immediately.
In other words,
git blame gcc/ChangeLog
is virtually useless because it's too expensive, but try doing
git gui blame gcc ChangeLog
instead, and doesn't that just seem nicer? (*)
The difference is that the GUI one does it incrementally, and doesn't have
to get _all_ the results before it can start reporting blame.
Not that I claim that the gui blame is perfect either (I dunno why it
delays the nice coloring so long, for example), but it was something I
pushed - and others made the gui for - exactly to help people with the
fact that git interally really does it that incremental way.
> SVN had the same problem (the file retrieval was the most expensive op
> on FSFS). One of the things i did to speed it up tremendously was to
> do the annotate from newest to oldest (IE in reverse), and stop
> annotating when we had come up with annotate info for all the lines.
We do that. The expense for git is that we don't do the revisions as a
single file at all. We'll look through each commit, check whether the
"gcc" directory changed, if it did, we'll go into it, and check whether
the "ChangeLog" file changed - and if it did, we'll actually diff it
against the previous version.
> In GCC history, it is likely you will be able to cut off at least 30%
> of the time if you do this, because files often have changed entirely
> multiple times.
Not gcc/ChangeLog, though (apart from the renames that happen
occasionally).
Btw, an example of something git *should* do right, but is just too damn
expensive, is doing
git gui blame gcc/ChangeLog-2000
and have it actually be able to track the original source of each of those
annotations across that "ChangeLog split from hell".
I bet it would eventually get it right, but that's a large file, way back
in history, and it will try to do a non-whitespace blame with copy
detection.
That's *expensive*, although it is an amusing thing to try to do ;)
Linus
PS. I also do agree that we seem to use an excessive amount of memory
there. As to whether it's the same issue or not, I'd not go as far as Nico
and say "yes" yet. But it's interesting.
It's not entirely surprising that we see multiple issues with the gcc
repo, simply because it's not the kind of repo that people have ever
really worked on. So I don't think it's necessarily related at all, except
in the sense of it being a different load and showing issues.
^ permalink raw reply
* Re: Something is broken in repack
From: Junio C Hamano @ 2007-12-11 19:40 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jon Smirl, Nicolas Pitre, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712111055590.25032@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, 11 Dec 2007, Jon Smirl wrote:
>> >
>> > So if you want to use more threads, that _forces_ you to have a bigger
>> > memory footprint, simply because you have more "live" objects that you
>> > work on. Normally, that isn't much of a problem, since most source files
>> > are small, but if you have a few deep delta chains on big files, both the
>> > delta chain itself is going to use memory (you may have limited the size
>> > of the cache, but it's still needed for the actual delta generation, so
>> > it's not like the memory usage went away).
>>
>> This makes sense. Those runs that blew up to 4.5GB were a combination
>> of this effect and fragmentation in the gcc allocator. Google
>> allocator appears to be much better at controlling fragmentation.
>
> Yes. I think we do have some case where we simply keep a lot of objects
> around, and if we are talking reasonably large deltas, we'll have the
> whole delta-chain in memory just to unpack one single object.
Eh, excuse me. unpack_delta_entry()
- first unpacks the base object (this goes recursive);
- uncompresses the delta;
- applies the delta to the base to obtain the target object;
- frees delta;
- frees (but allows it to be cached) the base object;
- returns the result
So no matter how deep a chain is, you keep only one delta at a time in
core, not whole delta-chain in core.
> So what you actually want to do is to just re-use already packed delta
> chains directly, which is what we normally do. But you are explicitly
> looking at the "--no-reuse-delta" (aka "git repack -f") case, which is why
> it then blows up.
While that does not explain, as Nico pointed out, the huge difference
between the two repack runs that have different starting pack, I would
say it is a fair thing to say. If you have a suboptimal pack (i.e. not
enough reusable deltas, as in the 2.1GB pack case), do run "repack -f",
but if you have a good pack (i.e. 300MB pack), don't.
^ 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