* Re: [PATCH] Fix git_setup_directory_gently when GIT_DIR is set
From: Junio C Hamano @ 2006-06-06 1:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <7vfyij2mo8.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>> Hmph. Would it be a bug in clone that does not create GIT_DIR
>>> then?
>>
>> I don't think so. The whole point in calling git-init-db is to create
>> that. GIT_DIR is set so that the otherwise nice work-in-a-subdirectory
>> does not kick in. Imagine for example:
>>
>> git-clone ./. victim
>>
>> (taken straight out of t5400). If GIT_DIR was not set, git-init-db (which
>> reads repositoryformat from the config if that exists, right?) would find
>> .git/ in git/t/trash, and _not_ create git/t/trash/victim/.git/.
>
> I know clone currently relies on init-db to create the directory if
> it does not exist (I wrote the code after all).
Ah, I think I see the real problem is. Alias handling is done
too early, and for commands like init-db that does _not_ even
want to look at an existing repository it tries to use GIT_DIR.
So how about this patch instead on top of yours?
-- >8 --
git alias: try alias last.
This disables alias "foo" from being used for git-foo, and when
we do use alias we check the built-in and then existing command
names first and then alias as the fallback. This avoids the
problem of common commands used in scripts getting clobbered by
user specific aliases.
---
diff --git a/git.c b/git.c
index 8854472..6db8f2b 100644
--- a/git.c
+++ b/git.c
@@ -202,6 +202,7 @@ int main(int argc, const char **argv, ch
char *slash = strrchr(cmd, '/');
char git_command[PATH_MAX + 1];
const char *exec_path = NULL;
+ int done_alias = 0;
/*
* Take the basename of argv[0] as the command
@@ -229,7 +230,6 @@ int main(int argc, const char **argv, ch
if (!strncmp(cmd, "git-", 4)) {
cmd += 4;
argv[0] = cmd;
- handle_alias(&argc, &argv);
handle_internal_command(argc, argv, envp);
die("cannot handle %s internally", cmd);
}
@@ -287,13 +287,21 @@ int main(int argc, const char **argv, ch
exec_path = git_exec_path();
prepend_to_path(exec_path, strlen(exec_path));
- handle_alias(&argc, &argv);
+ while (1) {
+ /* See if it's an internal command */
+ handle_internal_command(argc, argv, envp);
- /* See if it's an internal command */
- handle_internal_command(argc, argv, envp);
+ /* .. then try the external ones */
+ execv_git_cmd(argv);
- /* .. then try the external ones */
- execv_git_cmd(argv);
+ /* It could be an alias -- this works around the insanity
+ * of overriding "git log" with "git show" by having
+ * alias.log = show
+ */
+ if (done_alias || !handle_alias(&argc, &argv))
+ break;
+ done_alias = 1;
+ }
if (errno == ENOENT)
cmd_usage(0, exec_path, "'%s' is not a git-command", cmd);
^ permalink raw reply related
* Re: Horrible re-packing?
From: Linus Torvalds @ 2006-06-06 0:35 UTC (permalink / raw)
To: Chris Wedgwood
Cc: Nicolas Pitre, Olivier Galibert, Junio C Hamano, Git Mailing List
In-Reply-To: <20060606001802.GB5401@tuatara.stupidest.org>
On Mon, 5 Jun 2006, Chris Wedgwood wrote:
>
> On Mon, Jun 05, 2006 at 05:22:02PM -0400, Nicolas Pitre wrote:
>
> > Much more expensive for both memory usage and CPU cycles.
>
> On a modern CPU I'm not sure you would notice unless the dataset was
> insanely large.
The dataset really _is_ pretty large.
For the kernel, we're talking about a quarter million strings, and it's
not shrinking. Other (CVS imported from long histories) are in the several
million object range.
The real problem, btw, is not the CPU cost of sorting them (hey, O(nlogn)
works ok), but the memory use. We have to keep those quarter million names
in memory too. At a "pointer + average of ~30 bytes of full pathname
allocation + malloc overhead", the strings would basically take about 40
bytes of memory each, and cause constant cache-misses. In contrast, the
"hash" value is a 32-bit unsigned, with no pointer overhead.
It's not the biggest part to keep track of (we need to obviously keep
track of the 20-byte SHA1 and the pointers between objects), but if we had
the full string info, it would be quite noticeable overhead, on the order
of several tens of megabytes.
Now, "tens of megabytes" is not a make-or-break issue any more (you
definitely want lots of memory if you want to develop with large trees),
but in a very real sense, the memory footprint of an app is often very
closely correlated with its performance these days.
Trying to keep things within the L2 cache can help a lot, and even if we
expect 2MB and 4MB L2's to get more and more common, it means that we
should _strive_ to keep datasets down. As it is, we have no _chance_ of
staying in the L2 cache on the kernel, but for smaller projects we
hopefully can still do so ;)
Linus
^ permalink raw reply
* Re: Horrible re-packing?
From: Chris Wedgwood @ 2006-06-06 0:18 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Olivier Galibert, Linus Torvalds, Junio C Hamano,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051721180.24152@localhost.localdomain>
On Mon, Jun 05, 2006 at 05:22:02PM -0400, Nicolas Pitre wrote:
> Much more expensive for both memory usage and CPU cycles.
On a modern CPU I'm not sure you would notice unless the dataset was
insanely large.
^ permalink raw reply
* Re: Horrible re-packing?
From: Junio C Hamano @ 2006-06-06 0:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7v8xob2m6j.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> - The output from rev-list --objects is fed to the function as
> its name parameter while path is set to NULL. When we allow
> a thin pack to be generated, rev-list --objects output also
> contains "-<commit-object-name>" lines. We read trees for
> these commits that are not going to be sent but can be used
> as base objects, and pass the pathname discovered from the
> tree using path and name pair (path is set to the linked list
> of struct name_path that describes the dirname, and name is
> set to the basename). This was done to reduce the need for
> allocating and copying the pathname in preparation for
> calling name_hash() function.
but it can trivially fixed by doing something like this.
---
diff --git a/pack-objects.c b/pack-objects.c
index 3590cd5..e0328f8 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -463,17 +463,10 @@ static void rehash_objects(void)
}
}
-struct name_path {
- struct name_path *up;
- const char *elem;
- int len;
-};
-
#define DIRBITS 12
-static unsigned name_hash(struct name_path *path, const char *name)
+static unsigned name_hash(const char *name)
{
- struct name_path *p = path;
const char *n = name + strlen(name);
unsigned hash = 0, name_hash = 0, name_done = 0;
@@ -492,14 +485,6 @@ static unsigned name_hash(struct name_pa
name_hash = hash;
hash = 0;
}
- for (p = path; p; p = p->up) {
- hash = hash * 11 + '/';
- n = p->elem + p->len;
- while (p->elem <= --n) {
- unsigned char c = *n;
- hash = hash * 11 + c;
- }
- }
/*
* Make sure "Makefile" and "t/Makefile" are hashed separately
* but close enough.
@@ -686,9 +671,9 @@ static int name_cmp_len(const char *name
}
static void add_pbase_object(struct tree_desc *tree,
- struct name_path *up,
const char *name,
- int cmplen)
+ int cmplen,
+ const char *fullname)
{
struct name_entry entry;
@@ -702,13 +687,12 @@ static void add_pbase_object(struct tree
sha1_object_info(entry.sha1, type, &size))
continue;
if (name[cmplen] != '/') {
- unsigned hash = name_hash(up, name);
+ unsigned hash = name_hash(fullname);
add_object_entry(entry.sha1, hash, 1);
return;
}
if (!strcmp(type, tree_type)) {
struct tree_desc sub;
- struct name_path me;
struct pbase_tree_cache *tree;
const char *down = name+cmplen+1;
int downlen = name_cmp_len(down);
@@ -719,10 +703,7 @@ static void add_pbase_object(struct tree
sub.buf = tree->tree_data;
sub.size = tree->tree_size;
- me.up = up;
- me.elem = entry.path;
- me.len = entry.pathlen;
- add_pbase_object(&sub, &me, down, downlen);
+ add_pbase_object(&sub, down, downlen, fullname);
pbase_tree_put(tree);
}
}
@@ -778,14 +759,14 @@ static void add_preferred_base_object(ch
for (it = pbase_tree; it; it = it->next) {
if (cmplen == 0) {
- hash = name_hash(NULL, "");
+ hash = name_hash("");
add_object_entry(it->pcache.sha1, hash, 1);
}
else {
struct tree_desc tree;
tree.buf = it->pcache.tree_data;
tree.size = it->pcache.tree_size;
- add_pbase_object(&tree, NULL, name, cmplen);
+ add_pbase_object(&tree, name, cmplen, name);
}
}
}
@@ -1328,7 +1309,7 @@ int main(int argc, char **argv)
}
if (get_sha1_hex(line, sha1))
die("expected sha1, got garbage:\n %s", line);
- hash = name_hash(NULL, line+41);
+ hash = name_hash(line+41);
add_preferred_base_object(line+41, hash);
add_object_entry(sha1, hash, 0);
}
^ permalink raw reply related
* Re: Fix typo in tutorial-2.txt
From: Linus Torvalds @ 2006-06-05 23:58 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Johannes Schindelin, Junio C Hamano, Git Mailing List
In-Reply-To: <20060605234441.GB14993@fieldses.org>
On Mon, 5 Jun 2006, J. Bruce Fields wrote:
>
> But don't commit SHA1's, e.g., depend on times, authors, etc., that we
> can't count on being the same?
You can set all these by hand with the
GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL,DATE}
variables. In fact, the test script library already does that for authors
(although not for dates, I think) to get consistent answers.
Linus
^ permalink raw reply
* Re: Horrible re-packing?
From: Junio C Hamano @ 2006-06-05 23:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606051256280.5498@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Mon, 5 Jun 2006, Junio C Hamano wrote:
>>
>> IIRC, sometimes this function is called with path and name split
>> and sometimes with full path in name
>
> Yeah, I was pretty confused by the whole hashing thing. Are you sure that
> complexity is needed, it seems a bit overkill.
Two issues in the code confuses any reader of that function.
- The code wants to hash Makefile from different revisions
together, and Makefile and t/Makefile close to each other.
The current code did it by treating '/' specially, used
basename hash as the upper bits of the resulting hash and
dirname hash as the lower bits. It's my tendency to treat
slashes specially too much, which is one of your favorite
things to pick me on.
This is not needed by your change anymore -- by only using
the tail of the filename, and making sure tail part weighs
more in the resulting group number, the new code gives the
desired grouping characteristics in a much simpler way.
- The output from rev-list --objects is fed to the function as
its name parameter while path is set to NULL. When we allow
a thin pack to be generated, rev-list --objects output also
contains "-<commit-object-name>" lines. We read trees for
these commits that are not going to be sent but can be used
as base objects, and pass the pathname discovered from the
tree using path and name pair (path is set to the linked list
of struct name_path that describes the dirname, and name is
set to the basename). This was done to reduce the need for
allocating and copying the pathname in preparation for
calling name_hash() function.
If you use only the "name" variable in your group number
computation, and suppose we are doing send-pack to send
updates between rev A..B, contrib/git-svn/Makefile from rev B
will use git-svn/Makefile (tail 16 characters) to compute the
number, but the blob from rev A (which we are not going to
send but would want to use as a potential delta base) will
have contrib/git-svn part in "path" (the element points at
string "git-svn", and its uplink points at another element
that points at "contrib" with an uplink that says it is at
the root level), and Makefile in "name". They will be hashed
slightly differently.
^ permalink raw reply
* Re: Fix typo in tutorial-2.txt
From: J. Bruce Fields @ 2006-06-05 23:44 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0606060111570.25344@wbgn013.biozentrum.uni-wuerzburg.de>
On Tue, Jun 06, 2006 at 01:15:32AM +0200, Johannes Schindelin wrote:
> On Mon, 5 Jun 2006, Linus Torvalds wrote:
>
> > I didn't actually _test_ the tutorial, but if the old command worked,
> > something is really wrong!
>
> Maybe some new gitter wants to get dirty hands? We used to have a test
> which just replayed the commands in the tutorial, to make sure that new
> users would not hit a regression or a syntax change...
Might be a nice idea.
But don't commit SHA1's, e.g., depend on times, authors, etc., that we
can't count on being the same? So you'd have to be clever to deal with
stuff like
git cat-file commit a344bd52
Also, note that I didn't even attempt to keep a consistent scenario
throughout the first tutorial.
--b.
^ permalink raw reply
* Re: [PATCH] Fix git_setup_directory_gently when GIT_DIR is set
From: Junio C Hamano @ 2006-06-05 23:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606060117180.25685@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Hmph. Would it be a bug in clone that does not create GIT_DIR
>> then?
>
> I don't think so. The whole point in calling git-init-db is to create
> that. GIT_DIR is set so that the otherwise nice work-in-a-subdirectory
> does not kick in. Imagine for example:
>
> git-clone ./. victim
>
> (taken straight out of t5400). If GIT_DIR was not set, git-init-db (which
> reads repositoryformat from the config if that exists, right?) would find
> .git/ in git/t/trash, and _not_ create git/t/trash/victim/.git/.
I know clone currently relies on init-db to create the directory if
it does not exist (I wrote the code after all).
I am questioning if that was a wise thing to do. In the case of
clone, we _know_ where we want the directory to be, so creating
the directory upfront before calling init-db feels like the
right thing to do. In all the case other than this "clone calls
init-db" I can think of, if we have GIT_DIR set and it is set to
a non-existent location, it would be a bug in the code/script
and I think it is saner to error out in such a case.
^ permalink raw reply
* Re: Fix typo in tutorial-2.txt
From: J. Bruce Fields @ 2006-06-05 23:38 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051245470.5498@g5.osdl.org>
On Mon, Jun 05, 2006 at 12:47:49PM -0700, Linus Torvalds wrote:
>
> This should be obvious enough.
>
> I didn't actually _test_ the tutorial, but if the old command worked,
> something is really wrong!
Aie, sorry, thanks.--b.
^ permalink raw reply
* Re: [PATCH] Fix git_setup_directory_gently when GIT_DIR is set
From: Johannes Schindelin @ 2006-06-05 23:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk67v2o85.fsf@assigned-by-dhcp.cox.net>
Hi,
On Mon, 5 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Hi,
> >
> > On Mon, 5 Jun 2006, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > When calling git_setup_directory_gently, and GIT_DIR was set, it just
> >> > ignored the variable nongit_ok.
> >>
> >> Hmph. Is this really a breakage? That is, gently() is meant
> >> for a case where you do not know if you even find a git
> >> repository and tell it not to complain because you are prepared
> >> for the case where you are not in a git repository.
> >
> > Yes, it is a breakage: in git-clone, line 212, we explicitely set GIT_DIR
> > (to the not-yet-existing repository path), and call git-init-db. Now, with
> > the alias thing we need to get the config if it exists, so we _got_ to
> > call gently(). Boom.
>
> Hmph. Would it be a bug in clone that does not create GIT_DIR
> then?
I don't think so. The whole point in calling git-init-db is to create
that. GIT_DIR is set so that the otherwise nice work-in-a-subdirectory
does not kick in. Imagine for example:
git-clone ./. victim
(taken straight out of t5400). If GIT_DIR was not set, git-init-db (which
reads repositoryformat from the config if that exists, right?) would find
.git/ in git/t/trash, and _not_ create git/t/trash/victim/.git/.
Ciao,
Dscho
^ permalink raw reply
* Re: Fix typo in tutorial-2.txt
From: Johannes Schindelin @ 2006-06-05 23:15 UTC (permalink / raw)
To: Linus Torvalds; +Cc: J. Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051245470.5498@g5.osdl.org>
Hi,
On Mon, 5 Jun 2006, Linus Torvalds wrote:
> I didn't actually _test_ the tutorial, but if the old command worked,
> something is really wrong!
Maybe some new gitter wants to get dirty hands? We used to have a test
which just replayed the commands in the tutorial, to make sure that new
users would not hit a regression or a syntax change...
Ciao,
Dscho
^ permalink raw reply
* Re: Horrible re-packing?
From: Nicolas Pitre @ 2006-06-05 23:13 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051432270.5498@g5.osdl.org>
On Mon, 5 Jun 2006, Linus Torvalds wrote:
>
>
> On Mon, 5 Jun 2006, Nicolas Pitre wrote:
> >
> > In other words, the pack shrunk to less than half the size of the
> > previous one !
>
> Ok, that's a bit more extreme than expected.
>
> It's obviously great news, and says that the approach of sorting by
> "reversed name" is a great heuristic, but at the same time it makes me
> worry a bit that this thing that is supposed to be a heuristic ends up
> being _so_ important from a pack size standpoint. I was happier when it
> was more about saving a couple of percent.
Well... this is the repository that exhibited a repack regression a
while ago, going from something like ~40MB to ~160MB when Junio
initially added the directory in the name hash. No other popular
repositories had that problem.
Which is why I said this repo is particularly sensitive to heuristic
changes. So I wouldn't worry too much about your proposed patch making
it too great in this case. It certainly didn't cause any (significant)
regression overall which is what matters.
We already have surprizing results when combining two heuristics
together although if used separately they do worse. So trying to have
fallback/incremental heuristics is going to make things simply too
complicated for when it breaks. Better experiment with new ideas and
adopt them when they do a better job universally.
... which your proposed hashing change does.
Nicolas
^ permalink raw reply
* Re: [PATCH] Fix git_setup_directory_gently when GIT_DIR is set
From: Junio C Hamano @ 2006-06-05 23:10 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606060053440.25344@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Mon, 5 Jun 2006, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > When calling git_setup_directory_gently, and GIT_DIR was set, it just
>> > ignored the variable nongit_ok.
>>
>> Hmph. Is this really a breakage? That is, gently() is meant
>> for a case where you do not know if you even find a git
>> repository and tell it not to complain because you are prepared
>> for the case where you are not in a git repository.
>
> Yes, it is a breakage: in git-clone, line 212, we explicitely set GIT_DIR
> (to the not-yet-existing repository path), and call git-init-db. Now, with
> the alias thing we need to get the config if it exists, so we _got_ to
> call gently(). Boom.
Hmph. Would it be a bug in clone that does not create GIT_DIR
then?
^ permalink raw reply
* Re: [PATCH] Fix git_setup_directory_gently when GIT_DIR is set
From: Johannes Schindelin @ 2006-06-05 22:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodx74ca9.fsf@assigned-by-dhcp.cox.net>
Hi,
On Mon, 5 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > When calling git_setup_directory_gently, and GIT_DIR was set, it just
> > ignored the variable nongit_ok.
>
> Hmph. Is this really a breakage? That is, gently() is meant
> for a case where you do not know if you even find a git
> repository and tell it not to complain because you are prepared
> for the case where you are not in a git repository.
Yes, it is a breakage: in git-clone, line 212, we explicitely set GIT_DIR
(to the not-yet-existing repository path), and call git-init-db. Now, with
the alias thing we need to get the config if it exists, so we _got_ to
call gently(). Boom.
Ciao,
Dscho
^ permalink raw reply
* Re: Horrible re-packing?
From: Linus Torvalds @ 2006-06-05 21:40 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051637490.24152@localhost.localdomain>
On Mon, 5 Jun 2006, Nicolas Pitre wrote:
>
> In other words, the pack shrunk to less than half the size of the
> previous one !
Ok, that's a bit more extreme than expected.
It's obviously great news, and says that the approach of sorting by
"reversed name" is a great heuristic, but at the same time it makes me
worry a bit that this thing that is supposed to be a heuristic ends up
being _so_ important from a pack size standpoint. I was happier when it
was more about saving a couple of percent.
Now, your repo may be a strange case, and it just happens to fit the
suggested hash, but on the other hand it's nice to see three totally
different repositories that all improve, albeit with wildly different
numbers.
I'm wondering if we could have some "incremental optimizer" thing that
would take a potentially badly packed archive, and just start looking for
better delta chain possibilities? That way we would still try to get a
good initial pack with some heuristic, but we could have people run the
incremental improver every once in a while looking for good deltas that it
missed due to the project not fitting the heuristics..
The fact that we normally do incremental repacking (and "-f" is unusual)
is obviously one thing that makes us less susceptible to bad patterns (and
is also what allows us to run the incremental optimizer - any good delta
choice will automatically percolate into subsequent versions, including
packs that have been cloned).
So the packing strategy itself seems to be very stable (and partly _due_
to the "optimization" to re-use earlier pack choices), but we currently
lack the thing that fixes up any initial bad assumptions in case they
happen.
Linus
^ permalink raw reply
* Re: Horrible re-packing?
From: Linus Torvalds @ 2006-06-05 21:27 UTC (permalink / raw)
To: Olivier Galibert; +Cc: Junio C Hamano, Nicolas Pitre, Git Mailing List
In-Reply-To: <20060605211436.GA58708@dspnet.fr.eu.org>
On Mon, 5 Jun 2006, Olivier Galibert wrote:
>
> Why don't you just sort the full path+filename with a strcmp variant
> that starts by the end of the string for comparison? May at least be
> simpler to understand.
That's actually what I was going to do, but we don't save the whole name,
just the sorting number.
(This is actually an area where saving space is important - we can easily
be working with hundreds of thousands or millions of objects, and we don't
want to keep the name of each of them around).
So the suggested hash sort is designed exactly to end up approximating
that ascii sort-from-end-of-string.
Linus
^ permalink raw reply
* Re: Horrible re-packing?
From: Nicolas Pitre @ 2006-06-05 21:22 UTC (permalink / raw)
To: Olivier Galibert; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <20060605211436.GA58708@dspnet.fr.eu.org>
On Mon, 5 Jun 2006, Olivier Galibert wrote:
> On Mon, Jun 05, 2006 at 12:03:31PM -0700, Linus Torvalds wrote:
> > Comments?
>
> Why don't you just sort the full path+filename with a strcmp variant
> that starts by the end of the string for comparison? May at least be
> simpler to understand.
Much more expensive for both memory usage and CPU cycles.
Nicolas
^ permalink raw reply
* Re: Horrible re-packing?
From: Nicolas Pitre @ 2006-06-05 21:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051155000.5498@g5.osdl.org>
On Mon, 5 Jun 2006, Linus Torvalds wrote:
>
>
> On this same thread..
>
> This trivial patch not only simplifies the name hashing, it actually
> improves packing for both git and the kernel.
>
> The git archive pack shrinks from 6824090->6622627 bytes (a 3%
> improvement), and the kernel pack shrinks from 108756213 to 108219021 (a
> mere 0.5% improvement, but still, it's an improvement from making the
> hashing much simpler!)
OK here's the scoop. I still have a sample repo (I forget who it was
from) that used to exhibit a big packing size regression which was fixed
a while ago. I tend to test new packing strategies on that repo as well
since it has rather interesting characteristics that makes it pretty
sensitive to changes to name hashing and size filtering heuristics.
Before this hashing patch (including the rev-list fix):
$ git repack -a -f
Generating pack...
Done counting 46391 objects.
Deltifying 46391 objects.
100% (46391/46391) done
Writing 46391 objects.
100% (46391/46391) done
Total 46391, written 46391 (delta 7457), reused 38934 (delta 0)
Pack pack-7f766f5af5547554bacb28c0294bd562589dc5e7 created.
$ ll .git/objects/pack/pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
-rw-rw-r-- 1 nico nico 39486095 Jun 5 16:28 .git/objects/pack/pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
Now with this patch applied:
$ git repack -a -f
Generating pack...
Done counting 46391 objects.
Deltifying 46391 objects.
100% (46391/46391) done
Writing 46391 objects.
100% (46391/46391) done
Total 46391, written 46391 (delta 9920), reused 36447 (delta 0)
Pack pack-7f766f5af5547554bacb28c0294bd562589dc5e7 created.
$ ll .git/objects/pack/pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
-rw-rw-r-- 1 nico nico 16150417 Jun 5 16:31 .git/objects/pack/pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
In other words, the pack shrunk to less than half the size of the
previous one !
And yes fsck-objects still pass (I was doubtful at first).
Nicolas
^ permalink raw reply
* Re: Horrible re-packing?
From: Olivier Galibert @ 2006-06-05 21:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Nicolas Pitre, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0606051155000.5498@g5.osdl.org>
On Mon, Jun 05, 2006 at 12:03:31PM -0700, Linus Torvalds wrote:
> Comments?
Why don't you just sort the full path+filename with a strcmp variant
that starts by the end of the string for comparison? May at least be
simpler to understand.
OG.
^ permalink raw reply
* Re: Gitk feature - show nearby tags
From: Jonas Fonseca @ 2006-06-05 21:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Mackerras, git
In-Reply-To: <7vejy48wp5.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote Sun, Jun 04, 2006:
> I do not necessarily think an ascii-art is needed, nor an
> appropriate way to present it to the curses user.
With certain limitations, I think it could be useful for some
epositories. Optionally, of course.
> When the user wants to "view" a commit, you could show from which
> branch heads and from which tags the commit is reachable, and perhaps
> which tag is the latest among the ones reachable from that commit, as
> part of the commit detail information you display on the lower pane
> (log/diff view).
Thanks for recapping, I've added this to the TODO file.
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH] A Perforce importer for git.
From: Alex Riesen @ 2006-06-05 21:00 UTC (permalink / raw)
To: Sean; +Cc: git
In-Reply-To: <20060604100430.cb2789dd.seanlkml@sympatico.ca>
Sean, Sun, Jun 04, 2006 16:04:30 +0200:
> > I'm rather looking for a ability to manage a single branch where
> > import "sync" events appear as a merge of changes to the files
> > involved in the sync. I just haven't figured out yet how to "break" a
> > Perforce change into changes to single files and import that broken up
> > commit into git as a merge.
>
> Ahh, so what you're really asking is for a way to maintain the perforce
> merge history within git. Whereas the current p4import script just
> shows a linear history without any merges.
I assume that by "perforce merge" you understand the set of revisions
in the working directory. That's what you get in a typical corporate
environment with hundreds libraries and source files somehow stitched
together in a hope it'd work. It does, surprisingly often. There is
also another "merge" in perforce workflow - plain text merge of many
files, done manually in working directory and checked in afterwards.
I believe it wouldn't be possible to get a history of this merge,
because there is just no information about the merge anywhere.
> The problem is that Perforce doesn't merge at the commit level. It
> allows changes from other branches to be pulled one file at a time and
> from any rev level.
Right. Awkward.
> Now, even if you break those changes into one git commit per file per
> revision level (yuck!), you still couldn't use them to record Perforce
> merges. Git would still merge the entire history of such commits from
> the other branch whenever you tried to merge just one.
I think it's worse: you can't merge (as in git) anything because of
that salad from local (working) and remote (p4 server-side) pathnames.
> AFAICS, the best you could do would be to create cherry-picks, plucking
> just the commits from the other branch you want. However at that point
> you're not getting a git merge anyway and it doesn't seem to be any
> benefit beyond what the importer already does. Well, the importer
> _could_ make a comment in the commit message describing where each
> file change originated (ie. from which branch/rev). Would that help?
Don't think so, even if this is surely better detailed this way. I
still wont have the ability to merge branches. Maybe if every change
to every file gets it own commit one can use that information to
either cherry-pick the changes or fix the pathnames and apply that
patch? And a P4 change could be represented as a git-merge.
Like this:
P4:
Change 213412
a/foo.c #3
b/bar.c #6
Git:
+--commit abcdef ----+
| a/foo.c +3 lines |
base-+ commit deffff (merge, represents Change 213412)
| commit abcccd |
+--b/bar.c -3 lines -+
Now I can cherry-pick (or just copy) commit "abcdef" or commit
"abcccd", and still can find out what that "Change 213412" was all
about.
^ permalink raw reply
* Re: Gitk feature - show nearby tags
From: Jonas Fonseca @ 2006-06-05 20:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Mackerras, git
In-Reply-To: <7v3bek7589.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote Sun, Jun 04, 2006:
> Jonas Fonseca <fonseca@diku.dk> writes:
>
> >> - I want to see the neighbouring commits, but UP or DOWN
> >> does not do what I naïvely expect. It scrolls the lower
> >> pane. I say TAB to go up.
> >
> > I wonder what tig version you are using. If you are using the tig
> > version from my git repo this should also be working to your
> > expectation, making ...
>
> Whichever was the latest when I wrote the message. I see you
> have added a handful commits on it since then.
Yes, I added your Makefile patch. Thanks for that one.
> >> - Press UP or DOWN and I can move the highlight to
> >> neighbouring commits. This is wonderful, but the lower
> >> pane does not follow this -- it keeps showing the original
> >> commit, and I have to say ENTER again.
> >
> > .. this unnecessary.
>
> Maybe I am misusing it then.
I admit the basic user controls might still have some rough spots.
Here is my take on what you are trying to do:
- When you open tig, UP/DOWN will move the cursor line.
- When you press ENTER on the commit you would like to see, it will
open the split view and move focus to the diff view. So if you press
Enter again it will start scrolling the diff view. This is much like
the way Mutt works.
- If you press UP/DOWN (while the diff view is focused) you will move
to previous or next commit in the main view (the one-line log view)
and load it in the diff view. That is, there is no reason to switch
to the main view unless you want to navigate to a commit without
repeatedly reloading the diff view which is clearly not what you are
requesting.
Is this not what you requested? That you can "see neighbouring commit"
by pressing UP and DOWN? But without having to press ENTER again.
Now, I've experienced some problems with ncurses and key detection but
only for Insert/Delete and Home/End. If UP/DOWN scrolls the diff view
something is terrible wrong.
> I like viewing the list in the upper and diff/log in the lower
> at the same time, and that is the primary reason I liked tig, so
> moving around in the commit list view and not seeing the
> diff/log updated in sync was major dissapointment at least for
> me.
Ok, well I can just make it optional, if you want the split view always
to be in sync, even while moving when the main view is in focus.
--
Jonas Fonseca
^ permalink raw reply
* Re: Using pickaxe to track changed symbol CR4_FEATURES_ADDR
From: Thomas Glanzmann @ 2006-06-05 20:16 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: Junio C Hamano, git
In-Reply-To: <20060605131151.b8878c7c.rdunlap@xenotime.net>
Hello Randy,
I confused it. CR4_FEATURES_ADDR was in the glue code of parallels
binary only modules, but it sounded like a a MMU definition. Thanks a
lot. That was why my pickaxe did not work out in the first place.
However I have a workaround for the damn build problem.
See http://wwwcip.informatik.uni-erlangen.de/~sithglan/parallels/
Thanks,
Thomas
^ permalink raw reply
* Re: Using pickaxe to track changed symbol CR4_FEATURES_ADDR
From: Randy.Dunlap @ 2006-06-05 20:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: sithglan, git
In-Reply-To: <7v8xob4bft.fsf@assigned-by-dhcp.cox.net>
On Mon, 05 Jun 2006 13:03:34 -0700 Junio C Hamano wrote:
> Thomas Glanzmann <sithglan@stud.uni-erlangen.de> writes:
>
> > I am looking for the symbol CR4_FEATURES_ADDR which must be gone in one
> > of the last kernel revision. Now how I do use pickaxe to track any
> > changes that involve my missing symbol? Or is there a better way to
> > track that change down?
>
> None of the major recent versions seem to have the symbol.
>
> : gitster; git grep -e CR4_FEATURES_ADDR \
> v2.6.12-rc2 v2.6.12 v2.6.13 v2.6.14 v2.6.15 \
> v2.6.16
>
> and I did not get any google hits for "CR4_FEATURES_ADDR". Are
> you spelling it right?
include/asm-i386/processor.h has names like:
/*
* Intel CPU features in CR4
*/
#define X86_CR4_VME 0x0001 /* enable vm86 extensions */
#define X86_CR4_PVI 0x0002 /* virtual interrupts flag enable */
#define X86_CR4_TSD 0x0004 /* disable time stamp at ipl 3 */
#define X86_CR4_DE 0x0008 /* enable debugging extensions */
#define X86_CR4_PSE 0x0010 /* enable page size extensions */
#define X86_CR4_PAE 0x0020 /* enable physical address extensions */
#define X86_CR4_MCE 0x0040 /* Machine check enable */
#define X86_CR4_PGE 0x0080 /* enable global pages */
#define X86_CR4_PCE 0x0100 /* enable performance counters at ipl 3 */
#define X86_CR4_OSFXSR 0x0200 /* enable fast FPU save and restore */
#define X86_CR4_OSXMMEXCPT 0x0400 /* enable unmasked SSE exceptions */
extern unsigned long mmu_cr4_features;
static inline void set_in_cr4 (unsigned long mask)
{
unsigned cr4;
mmu_cr4_features |= mask;
cr4 = read_cr4();
cr4 |= mask;
write_cr4(cr4);
}
static inline void clear_in_cr4 (unsigned long mask)
{
unsigned cr4;
mmu_cr4_features &= ~mask;
cr4 = read_cr4();
cr4 &= ~mask;
write_cr4(cr4);
}
but nothing exactly like you asked about.
---
~Randy
^ permalink raw reply
* Re: Gitk feature - show nearby tags
From: Junio C Hamano @ 2006-06-05 20:08 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Paul Mackerras, git
In-Reply-To: <7v3bek7589.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Jonas Fonseca <fonseca@diku.dk> writes:
>
> Whichever was the latest when I wrote the message. I see you
> have added a handful commits on it since then.
>
>>> - Press UP or DOWN and I can move the highlight to
>>> neighbouring commits. This is wonderful, but the lower
>>> pane does not follow this -- it keeps showing the original
>>> commit, and I have to say ENTER again.
>>
>> .. this unnecessary.
>
> Maybe I am misusing it then.
Sorry, I *was* misusing it. I was doing PageUp/PageDown. Duh.
I typed uparrow and downarrow while in the split view and it was
doing what I wanted to do.
^ 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