* [PATCH] git-upload-archive: add config option to allow only specified formats
From: Rene Scharfe @ 2006-09-10 15:58 UTC (permalink / raw)
To: Junio C Hamano, Franck Bui-Huu; +Cc: git
In-Reply-To: <7v1wqkt2v4.fsf_-_@assigned-by-dhcp.cox.net>
Documentation/config.txt | 5 +++++
builtin-upload-archive.c | 39 +++++++++++++++++++++++++++++++++++++++
daemon.c | 2 ++
3 files changed, 46 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce722a2..5c3c6c7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -236,6 +236,11 @@ tar.umask::
the same permissions as gitlink:git-checkout[1] would use. The default
value remains 0, which means world read-write.
+uploadarchive.daemonformats::
+ A comma-separated list of the git-archive formats allowed for upload
+ via git-daemon. If this parameter is missing all formats are allowed
+ for upload.
+
user.email::
Your email address to be recorded in any newly created commits.
Can be overridden by the 'GIT_AUTHOR_EMAIL' and 'GIT_COMMITTER_EMAIL'
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 96f96bd..6a5245a 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,37 @@ static const char upload_archive_usage[]
static const char deadchild[] =
"git-upload-archive: archiver died with error";
+static char *daemon_formats;
+
+static int upload_format_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "uploadarchive.daemonformats"))
+ daemon_formats = xstrdup(value);
+ return 0;
+}
+
+static int is_in(const char *needle, const char *haystack, const char *delim)
+{
+ int len = strlen(needle);
+ const char *search = haystack;
+
+ for (;;) {
+ char *pos = strstr(search, needle);
+ if (!pos)
+ return 0;
+ search++;
+ if ((pos == haystack || strchr(delim, pos[-1])) &&
+ (pos[len] == '\0' || strchr(delim, pos[len])))
+ return 1;
+ }
+}
+
+static int upload_format_allowed(const char *fmt)
+{
+ if (getenv("GIT_DAEMON"))
+ return daemon_formats ? is_in(fmt, daemon_formats, " \t,") : 1;
+ return 1;
+}
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
@@ -38,6 +69,8 @@ static int run_upload_archive(int argc,
if (!enter_repo(buf, 0))
die("not a git archive");
+ git_config(upload_format_config);
+
/* put received options in sent_argv[] */
sent_argc = 1;
sent_argv[0] = "git-upload-archive";
@@ -67,6 +100,12 @@ static int run_upload_archive(int argc,
/* parse all options sent by the client */
treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar);
+ if (!upload_format_allowed(ar.name)) {
+ free(daemon_formats);
+ die("upload of %s format forbidden\n", ar.name);
+ }
+ free(daemon_formats);
+
parse_treeish_arg(sent_argv + treeish_idx, &ar.args, prefix);
parse_pathspec_arg(sent_argv + treeish_idx + 1, &ar.args);
diff --git a/daemon.c b/daemon.c
index a2954a0..2d58abe 100644
--- a/daemon.c
+++ b/daemon.c
@@ -304,6 +304,8 @@ static int run_service(char *dir, struct
return -1;
}
+ setenv("GIT_DAEMON", "I am your father.", 1);
+
/*
* We'll ignore SIGTERM from now on, we have a
* good client.
^ permalink raw reply related
* Re: Change set based shallow clone
From: linux @ 2006-09-10 16:10 UTC (permalink / raw)
To: jonsmirl; +Cc: git, linux, paulus, torvalds
In-Reply-To: <9e4733910609100756r1ece1e22m38054536a2909dd4@mail.gmail.com>
> You noticed too that forks of small apps are relatively slow. The
> first pass of the import tools used fork for everything and took a
> week to run with 60% of the time spent in the kernel. There may be
> some work to do on fork in the kernel. Does mapping the kernel into
> the top 1G slow down fork of these small apps? Or are they dynamically
> linked to something that is bringing in millions of pages? When I was
> doing oprofile all of the time was in the actual fork call and page
> table copying.
Actually, Linux has one of the fastest forks around, 100-200 us on
modern x86. For large executables, the shared page tables patch (is it
merged yet?) might help.
No, mapping the kernel does NOT slow anything down. Those page tables are
shared between all processes, and use the x86's global bit to avoid being
forced out of the TLB during context switch. Attempting to make the
kernel mapping vary between processes would slow things down enormously!
I'm not finding reasonably recent Linux/FreeBSD comparisons. There are
plenty of Max OS X comparisons, but it appears to particularly suck,
so that's a bit of a straw man. See, e.g.
http://www.anandtech.com/mac/showdoc.aspx?i=2520&p=7
or some figures from 2002:
http://lists.apple.com/archives/darwin-kernel/2002/Oct/msg00009.html
----------------------------------------------------------------
Host OS Mhz null null open selct sig sig fork exec sh
call I/O stat clos TCP inst hndl proc proc proc
--------- ------------- ---- ---- ---- ---- ---- ----- ---- ---- ---- ---- ----
g4 Darwin 1.4 799 2.21 3.19 12.6 16.4 30.4 3.78 9.88 1576 4095 13.K
g4 Darwin 5.5 797 2.26 3.15 14.5 17.8 30.4 3.82 10.2 5685 12.K 40.K
g4.local. Darwin 6.0 797 1.47 2.73 17.2 20.7 27.2 3.00 10.5 7517 17.K 41.K
g4.local. Darwin 6.0 (after misconfiguration fixed) 1575
g4 Linux 2.4.18- 799 0.42 0.69 2.52 3.79 33.6 1.23 3.08 659. 2642 12.K
--------- ------------- ---- ---- ---- ---- ---- ----- ---- ---- ---- ---- ----
jim Darwin 1.4 448 2.85 4.23 9.53 17 4.83 14 2402 6817 19K
jim.local Darwin 6.1 448 1.89 3.71 21 29 43 3.85 15 2832 8955 19K
For small processes such as lmbench uses, you can see from the above that
exec is much more expensive than fork. Also
http://www.opersys.com/lrtbf/index.html
http://marc.theaimsgroup.com/?l=linux-kernel&m=112086443319815
http://www.opensourcetutorials.com/tutorials/Server-Side-Coding/Administration/hyper-threading-linux/page4.html
As for small apps and dynamic linking, glibc isn't tiny (prelink can
help), but that's post-exec. If it's in the fork() call, then it's just
a case that your app's memory space is big and even copying all of its
page tables is expensive.
If you're invoking one of the git programs that's a shell script, that
adds its own startup overhead, of course.
^ permalink raw reply
* [PATCH] Use xstrdup instead of strdup in builtin-{tar,zip}-tree.c
From: Rene Scharfe @ 2006-09-10 16:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Franck Bui-Huu, git
In-Reply-To: <7v8xksx1ww.fsf@assigned-by-dhcp.cox.net>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
This patch applies to the current 'next' branch.
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index c20eb0e..e8e492f 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -390,7 +390,7 @@ int write_tar_archive(struct archiver_ar
write_global_extended_header(args->commit_sha1);
if (args->base && plen > 0 && args->base[plen - 1] == '/') {
- char *base = strdup(args->base);
+ char *base = xstrdup(args->base);
int baselen = strlen(base);
while (baselen > 0 && base[baselen - 1] == '/')
diff --git a/builtin-zip-tree.c b/builtin-zip-tree.c
index 4e79633..fdac2bd 100644
--- a/builtin-zip-tree.c
+++ b/builtin-zip-tree.c
@@ -363,7 +363,7 @@ int write_zip_archive(struct archiver_ar
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
if (args->base && plen > 0 && args->base[plen - 1] == '/') {
- char *base = strdup(args->base);
+ char *base = xstrdup(args->base);
int baselen = strlen(base);
while (baselen > 0 && base[baselen - 1] == '/')
^ permalink raw reply related
* Re: [PATCH] git-upload-archive: add config option to allow only specified formats
From: Rene Scharfe @ 2006-09-10 16:12 UTC (permalink / raw)
To: Junio C Hamano, Franck Bui-Huu; +Cc: git
In-Reply-To: <20060910155837.GA15974@lsrfire.ath.cx>
*sigh* The patch contained in the parent message is:
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
^ permalink raw reply
* Re: [PATCH] git-upload-archive: add config option to allow only specified formats
From: Junio C Hamano @ 2006-09-10 18:00 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Franck Bui-Huu, git
In-Reply-To: <20060910155837.GA15974@lsrfire.ath.cx>
Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Documentation/config.txt | 5 +++++
> builtin-upload-archive.c | 39 +++++++++++++++++++++++++++++++++++++++
> daemon.c | 2 ++
> 3 files changed, 46 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ce722a2..5c3c6c7 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -236,6 +236,11 @@ tar.umask::
> the same permissions as gitlink:git-checkout[1] would use. The default
> value remains 0, which means world read-write.
>
> +uploadarchive.daemonformats::
> + A comma-separated list of the git-archive formats allowed for upload
> + via git-daemon. If this parameter is missing all formats are allowed
> + for upload.
> +
Fine -- do we have any other "list-ish" configuration variable,
by the way? I am just wondering if we earlier established a
convention to use some delimiter to list out things and if we do
have such a convention if delimiter is a comma or not.
> diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
> index 96f96bd..6a5245a 100644
> --- a/builtin-upload-archive.c
> +++ b/builtin-upload-archive.c
> @@ -16,6 +16,37 @@ static const char upload_archive_usage[]
> static const char deadchild[] =
> "git-upload-archive: archiver died with error";
>
> +static char *daemon_formats;
> +
> +static int upload_format_config(const char *var, const char *value)
> +{
> + if (!strcmp(var, "uploadarchive.daemonformats"))
> + daemon_formats = xstrdup(value);
> + return 0;
> +}
This let's the repository owner to decide what can be used.
> +static int upload_format_allowed(const char *fmt)
> +{
> + if (getenv("GIT_DAEMON"))
> + return daemon_formats ? is_in(fmt, daemon_formats, " \t,") : 1;
> + return 1;
> +}
And limits the allowed format when the environment set to the
value the repository owner decided.
> static int run_upload_archive(int argc, const char **argv, const char *prefix)
> {
> @@ -67,6 +100,12 @@ static int run_upload_archive(int argc,
> /* parse all options sent by the client */
> treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar);
>
> + if (!upload_format_allowed(ar.name)) {
> + free(daemon_formats);
> + die("upload of %s format forbidden\n", ar.name);
> + }
> + free(daemon_formats);
> +
So we could enhance "--remote --list" to show what are supported
(both codewise and policywise) on the remote end, with a bit of
code restructuring?
> diff --git a/daemon.c b/daemon.c
> index a2954a0..2d58abe 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -304,6 +304,8 @@ static int run_service(char *dir, struct
> return -1;
> }
>
> + setenv("GIT_DAEMON", "I am your father.", 1);
I suspect "upload_format_allowed()" can be taught to see what is
in this environment variable and sometimes take that as
daemon_format without letting the repository to override it, so
that the site administrator can limit the formats that can be
used further, just like daemon service mechanism lets them be in
control.
^ permalink raw reply
* Re: Change set based shallow clone
From: Jon Smirl @ 2006-09-10 18:00 UTC (permalink / raw)
To: linux@horizon.com; +Cc: git, paulus, torvalds
In-Reply-To: <20060910161007.8846.qmail@science.horizon.com>
On 10 Sep 2006 12:10:07 -0400, linux@horizon.com <linux@horizon.com> wrote:
> Actually, Linux has one of the fastest forks around, 100-200 us on
> modern x86. For large executables, the shared page tables patch (is it
> merged yet?) might help.
Here is the opfile of the kernel
3467889 18.9893 copy_page_range
2190416 11.9941 unmap_vmas
1156011 6.3300 page_fault
887794 4.8613 release_pages
860853 4.7138 page_remove_rmap
633243 3.4675 get_page_from_freelist
398773 2.1836 do_wp_page
344422 1.8860 __mutex_lock_slowpath
280070 1.5336 __handle_mm_fault
241713 1.3236 do_page_fault
238398 1.3054 __d_lookup
236654 1.2959 vm_normal_page
At the time this was measured parsecvs was executing millions of git
command using system(command). 40% of the CPU was in the kernel, it
stayed that way for hours.
18262372 41.0441 /home/good/vmlinux
5465741 12.2841 /usr/bin/cvs
4374336 9.8312 /lib/libc-2.4.so
3627709 8.1532 /lib/libcrypto.so.0.9.8a
2494610 5.6066 /usr/bin/oprofiled
2471238 5.5540 /usr/lib/libz.so.1.2.3
945349 2.1246 /usr/lib/perl5/5.8.8/i386-linux-thread-multi/CORE/libperl.so
933646 2.0983 /usr/local/bin/git-read-tree
758776 1.7053 /usr/local/bin/git-write-tree
642502 1.4440 /lib/ld-2.4.so
472903 1.0628 /nvidia
379254 0.8524 /usr/local/bin/git-pack-objects
Maybe we are looking at the wrong thing, it may be fast to fork a
process, is it fast for the process to exit?
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Dropping Git.pm (at least Git.xs)?
From: Sam Vilain @ 2006-09-10 17:59 UTC (permalink / raw)
To: Dennis Stosberg; +Cc: Junio C Hamano, pasky, git
In-Reply-To: <20060903150305.G50c94aea@leonov.stosberg.net>
Dennis Stosberg wrote:
> Having perl bindings to git internals and sometime in the future to a
> libified git is a great thing. It will allow people to do interesting
> things, quickly trying concepts without having to write any C code.
> And I expect that gitweb can be sped up remarkably by using Git.pm (no
> forking, parsing of command output often not necessary, easy caching of
> frequently cached data across calls, etc)
FWIW, I have been starting on a perl implementation. It uses the
Git.pm, but not for anything *that* important. It's still very young,
but once I have reading and writing files basically working, I'll
release it to CPAN separately - no reason it needs to be distributed
with Git itself.
See http://utsl.gen.nz/gitweb/?p=VCS-Git
I used this design to talk about Moose at YAPC::Europe 2006.
http://utsl.gen.nz/talks/moose/start.html
Sam.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-10 18:32 UTC (permalink / raw)
To: linux@horizon.com; +Cc: junkio, git, jonsmirl, paulus, torvalds
In-Reply-To: <20060910152122.31694.qmail@science.horizon.com>
>
> But can we stop designing git-rev-list output formats until we've figured
> out if and how to implement it in the visualizer? Or, more to the point,
> visualizers plural. That's the hard part. Then we can see what sort
> of git-rev-list output would be most convenient.
>
Regarding qgit we have two cases:
-arrive of fixup set for out of order commits _not_ currently
displayed on the screen
This is by far the most common case expecially for big archives. In
this case the fix speed depends on how far are the fixed commits from
the last loaded commit (it's an insertion in a vector). In any case
far more quick then redrawing the graph. No flickering side effect
here. Implementation is easy.
-arrive of fixup set for out of order commits currently displayed on the screen
Here implementation _could_ be more difficult, but given the very low
statistic of this case (a rare case among rare cases) we could accept
the brutal approach of reset the complete graph, *but not the loaded
revisions data*, and redraw the graph. With this approach
implementation is almost as easy as before but flickering is involved.
I agree with you that we should have fixup information as soon as
git-rev-list discovers out of order commits.
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-10 18:51 UTC (permalink / raw)
To: Jon Smirl; +Cc: git, Shawn Pearce
In-Reply-To: <9e4733910609100756r1ece1e22m38054536a2909dd4@mail.gmail.com>
"Jon Smirl" <jonsmirl@gmail.com> writes:
> Using the Mozilla repo you downloaded is not a normal situation since
> it is 100% packed. Most people are going to have a few thousand loose
> objects floating around too. Loose objects really slow things down.
When you have a few thousand loose objects you definitely should
consider repacking (not "repack -a" for Mozilla case, perhaps).
We could benefit from the suggested organization of one base
archive pack plus a current active pack. The core side code to
help doing so was posted here which followed a discussion on how
to have repack make use of it last week.
http://thread.gmane.org/gmane.comp.version-control.git/26218/focus=26326
Any takers?
^ permalink raw reply
* Re: [PATCH 1/2] archive: allow remote to have more formats than we understand.
From: Franck Bui-Huu @ 2006-09-10 19:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rene Scharfe
In-Reply-To: <7vpse4tcyc.fsf@assigned-by-dhcp.cox.net>
2006/9/10, Junio C Hamano <junkio@cox.net>:
> This fixes git-archive --remote not to parse archiver arguments;
> otherwise if the remote end implements formats other than the
> one known locally we will not be able to access that format.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
> * At first sight, this should not matter that much, but (1) we
> do not really parse and validate the arguments when dealing
> with remote site, and (2) we have no way validating them if
> we wanted to, given that the remote end might be running
> different version of git.
>
> Having said that, you would realize that once we start
> refactoring things this way, "git archive --remote=foo" is
> not archive driver anymore. There is nothing that prevents
> us saying "git archive --remote=foo --command=rev-list HEAD",
> other than that the remote archive protocol insists the
> command invoked at the remote end to be "git archive" itself.
>
good change.
> archive.h | 1 -
> builtin-archive.c | 79 ++++++++++++++++++++++++++++++++---------------------
> 2 files changed, 47 insertions(+), 33 deletions(-)
>
[snip]
> return i;
> }
>
> +static const char *remote_request(int *ac, const char **av)
> +{
just to be consistent with the rest of the file, I would have called
this function
"parse_remote_arg" or "extract_remote_arg"
--
Franck
^ permalink raw reply
* Re: [PATCH 1/2] archive: allow remote to have more formats than we understand.
From: Junio C Hamano @ 2006-09-10 19:07 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: Junio C Hamano, git, Rene Scharfe
In-Reply-To: <cda58cb80609101202y2ee1a18dwf09d6e104740777@mail.gmail.com>
"Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
>> +static const char *remote_request(int *ac, const char **av)
>> +{
>
> just to be consistent with the rest of the file, I would have called
> this function
> "parse_remote_arg" or "extract_remote_arg"
I was thinking about calling this is_remote_request() actually.
^ permalink raw reply
* Re: [PATCH] git-upload-archive: add config option to allow only specified formats
From: Franck Bui-Huu @ 2006-09-10 19:07 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <20060910155837.GA15974@lsrfire.ath.cx>
2006/9/10, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
> Documentation/config.txt | 5 +++++
> builtin-upload-archive.c | 39 +++++++++++++++++++++++++++++++++++++++
> daemon.c | 2 ++
> 3 files changed, 46 insertions(+)
[snip]
>
> + if (!upload_format_allowed(ar.name)) {
> + free(daemon_formats);
> + die("upload of %s format forbidden\n", ar.name);
> + }
just out of curiousity, why "free(daemon_formats)" right before a "die()" ?
--
Franck
^ permalink raw reply
* Re: [PATCH 1/3] Move sideband client side support into reusable form.
From: Franck Bui-Huu @ 2006-09-10 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rene Scharfe
In-Reply-To: <7vd5a4t3e0.fsf_-_@assigned-by-dhcp.cox.net>
2006/9/10, Junio C Hamano <junkio@cox.net>:
> This moves the receiver side of the sideband support from
> fetch-clone.c to sideband.c and its header file, so that
> archiver protocol can use it.
>
looks good, that's what I was doing but you beat me.
--
Franck
^ permalink raw reply
* Re: [PATCH 3/3] Add sideband status report to git-archive protocol
From: Franck Bui-Huu @ 2006-09-10 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rene Scharfe
In-Reply-To: <7v1wqkt2v4.fsf_-_@assigned-by-dhcp.cox.net>
2006/9/10, Junio C Hamano <junkio@cox.net>:
> Using the refactored sideband code from existing upload-pack protocol,
> this lets the error condition and status output sent from the remote
> process to be shown locally.
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
>
> * This does it in a "stupid" way. Ideally, write(2) to fd 1
> and 2 in each archiver backend could be wrapped with
> something like upload-pack.c::send_client_data() that does
> straight write(2) to the original destination when it is not
> driven by upload-pack, or use send_sideband() when it is, and
> that way we can lose two pipes and the multiplexer process.
>
Well I started that part the same way you do because I liked the fact
that archiver backend implementation needn't to know how to send out
data. It's totaly transparent for it. It just need to write its
payload on stdout.
>
> builtin-archive.c | 24 ++++++++++--
> builtin-upload-archive.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 108 insertions(+), 8 deletions(-)
>
[snip]
> +int cmd_upload_archive(int argc, const char **argv, const char *prefix)
> +{
> + pid_t writer;
> + int fd1[2], fd2[2];
> + /*
> + * Set up sideband subprocess.
> + *
> + * We (parent) monitor and read from child, sending its fd#1 and fd#2
> + * multiplexed out to our fd#1. If the child dies, we tell the other
> + * end over channel #3.
> + */
> + if (pipe(fd1) < 0 || pipe(fd2) < 0) {
> + int err = errno;
> + packet_write(1, "NACK pipe failed on the remote side\n");
> + die("upload-archive: %s", strerror(err));
> + }
> + writer = fork();
> + if (writer < 0) {
> + int err = errno;
> + packet_write(1, "NACK fork failed on the remote side\n");
> + die("upload-archive: %s", strerror(err));
> + }
> + if (!writer) {
> + /* child - connect fd#1 and fd#2 to the pipe */
> + dup2(fd1[1], 1);
> + dup2(fd2[1], 2);
> + close(fd1[1]); close(fd2[1]);
> + close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
> +
> + exit(run_upload_archive(argc, argv, prefix));
> + }
> +
> + /* parent - read from child, multiplex and send out to fd#1 */
> + close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
> packet_write(1, "ACK\n");
> packet_flush(1);
>
> - return ar.write_archive(&ar.args);
> -}
> + while (1) {
> + struct pollfd pfd[2];
> + char buf[16384];
> + ssize_t sz;
> + pid_t pid;
> + int status;
> +
> + pfd[0].fd = fd1[0];
> + pfd[0].events = POLLIN;
> + pfd[1].fd = fd2[0];
> + pfd[1].events = POLLIN;
> + if (poll(pfd, 2, -1) < 0) {
> + if (errno != EINTR) {
> + error("poll failed resuming: %s",
> + strerror(errno));
> + sleep(1);
> + }
> + continue;
> + }
> + if (pfd[0].revents & (POLLIN|POLLHUP)) {
> + /* Data stream ready */
> + sz = read(pfd[0].fd, buf, sizeof(buf));
> + send_sideband(1, 1, buf, sz, DEFAULT_PACKET_MAX);
> + }
> + if (pfd[1].revents & (POLLIN|POLLHUP)) {
> + /* Status stream ready */
> + sz = read(pfd[1].fd, buf, sizeof(buf));
> + send_sideband(1, 2, buf, sz, DEFAULT_PACKET_MAX);
> + }
>
> + if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0)
> + continue;
> + /* did it die? */
> + pid = waitpid(writer, &status, WNOHANG);
> + if (!pid) {
> + fprintf(stderr, "Hmph, HUP?\n");
> + continue;
> + }
> + if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
> + send_sideband(1, 3, deadchild, strlen(deadchild),
> + DEFAULT_PACKET_MAX);
> + packet_flush(1);
> + break;
> + }
> + return 0;
> +}
Why all of this is part of upload-archive ? Shouldn't we put that code
in daemon.c ?
We could use a new service flag to ask daemon.c to start the service
with the sideband multiplexer process already setup and plugged with
the sercive is going to start ? Hence others future services could use
it.
--
Franck
^ permalink raw reply
* Re: [PATCH 1/2] archive: allow remote to have more formats than we understand.
From: Franck Bui-Huu @ 2006-09-10 19:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Rene Scharfe
In-Reply-To: <7vwt8br150.fsf@assigned-by-dhcp.cox.net>
2006/9/10, Junio C Hamano <junkio@cox.net>:
> "Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
>
> >> +static const char *remote_request(int *ac, const char **av)
> >> +{
> >
> > just to be consistent with the rest of the file, I would have called
> > this function
> > "parse_remote_arg" or "extract_remote_arg"
>
> I was thinking about calling this is_remote_request() actually.
>
that sounds like to return a boolean. You would need to pass remote as
a parameter, no ?
I think extract_remote_arg is nice because it tells you that it
returns remote option valu _and_ remove it from argv.
--
Franck
^ permalink raw reply
* Re: Change set based shallow clone
From: linux @ 2006-09-10 19:03 UTC (permalink / raw)
To: jonsmirl, linux; +Cc: git, paulus, torvalds
In-Reply-To: <9e4733910609101100u515ae6e3seababd5780db715d@mail.gmail.com>
> Maybe we are looking at the wrong thing, it may be fast to fork a
> process, is it fast for the process to exit?
The lmbench benchmark figures I cited are for fork+exit. The other is
fork+exec+exit.
> At the time this was measured parsecvs was executing millions of git
> command using system(command). 40% of the CPU was in the kernel, it
> stayed that way for hours.
Ah! You are aware, aren't you, that system(string) invokes
/bin/sh to parse the string, right? So you're starting bash
several million times. ("man system" explains.)
A direct fork() (or even faster, vfork) and exec() is going to have a
lot less overhead, although it's more work to code. See Stevens for
excellent examples.
^ permalink raw reply
* Re: Change set based shallow clone
From: Linus Torvalds @ 2006-09-10 20:00 UTC (permalink / raw)
To: linux; +Cc: jonsmirl, git, paulus
In-Reply-To: <20060910190332.17667.qmail@science.horizon.com>
On Sun, 10 Sep 2006, linux@horizon.com wrote:
>
> A direct fork() (or even faster, vfork) and exec() is going to have a
> lot less overhead, although it's more work to code. See Stevens for
> excellent examples.
Well, that said, the Linux fork/exec/exit is certainly fairly efficient,
but nothing can hide the fact that it _is_ a very expensive operation.
So we may do a fork/exit in a millisecond, but on the other hand, we can
generally open a file in a microsecond. So we're definitely talking about
several orders of magnitude difference..
And a lot of the trivial git helpers are literally not doing a lot more
than opening a file, reading it, and parsing it. That's for things like
looking up the SHA1 of a branch head, for example. Doing it in-line might
be a microsecond or two, while spawning a shell to execute git-rev-parse
will take you several microseconds.
Do it a million times, and the difference is now a second vs an hour. Or a
few minutes vs a week.
So in that sense, it is absolutely undeniable that fork/exit is slow.
Everything is relative. The Linux fork/exit is fast when compared to most
other systems fork/exit, but it's slow as hell when compared to just doing
some small op directly in-line.
Linus
^ permalink raw reply
* Re: [PATCH 3/3] Add sideband status report to git-archive protocol
From: Junio C Hamano @ 2006-09-10 20:31 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: Junio C Hamano, git, Rene Scharfe
In-Reply-To: <cda58cb80609101215h36b0f7d7m990ed3011cfda9d2@mail.gmail.com>
"Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
> We could use a new service flag to ask daemon.c to start the service
> with the sideband multiplexer process already setup and plugged with
> the sercive is going to start ? Hence others future services could use
> it.
We do not know how "future services" would look like, so I do
not think it is worth doing so at this moment.
Refactoring existing code after the requirements of the second
and subsequent users are identified would result in much nicer
outcome with less effort, just like I showed here with the two
patches, than overengineering things in advance without knowing
what those requirements are to be generic enough.
For example, moving that code to reusable piece (to daemon.c or
elsewhere -- it does not make a difference) does not even help a
similar code that already exists in upload-pack.c, which needs
to monitor multiple processes involved in a pipe and report
failure exit of any of them. We would definitely want to have
something that can cover both of these existing cases when the
next user comes along, and if that next user fits in one of the
patterns (either have a pipeline with multiple processes like
upload-pack.c does and has to monitor all of them, or just one
process which is essentially an internal subroutine call and has
to monitor only that process) then refactoring this part right
now to cover both cases would make sense but we do not know what
that next user will look like.
^ permalink raw reply
* Re: Change set based shallow clone
From: Jon Smirl @ 2006-09-10 21:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux, git, paulus
In-Reply-To: <Pine.LNX.4.64.0609101254590.27779@g5.osdl.org>
On 9/10/06, Linus Torvalds <torvalds@osdl.org> wrote:
> On Sun, 10 Sep 2006, linux@horizon.com wrote:
> >
> > A direct fork() (or even faster, vfork) and exec() is going to have a
> > lot less overhead, although it's more work to code. See Stevens for
> > excellent examples.
>
> Well, that said, the Linux fork/exec/exit is certainly fairly efficient,
> but nothing can hide the fact that it _is_ a very expensive operation.
cvs2svn + fastimport can import the same Mozilla repo in about 2hrs
that was taking parsecvs about five days to do. The algorithms are not
that different, cvs2svn is probably slower than parsecvs for detecting
changesets. The difference is mostly due to the removal of forks.
Is the kernel mapped into user processes using huge pages? That would
reduce some of the pressure on TLBs. Another thing that should be
mapped with huge pages is the video RAM.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Dropping Git.pm (at least Git.xs)?
From: Jakub Narebski @ 2006-09-10 22:13 UTC (permalink / raw)
To: git
In-Reply-To: <4504529A.70401@vilain.net>
Sam Vilain wrote:
> Dennis Stosberg wrote:
>> Having perl bindings to git internals and sometime in the future to a
>> libified git is a great thing. It will allow people to do interesting
>> things, quickly trying concepts without having to write any C code.
>> And I expect that gitweb can be sped up remarkably by using Git.pm (no
>> forking, parsing of command output often not necessary, easy caching of
>> frequently cached data across calls, etc)
>
> FWIW, I have been starting on a perl implementation. It uses the
> Git.pm, but not for anything *that* important. It's still very young,
> but once I have reading and writing files basically working, I'll
> release it to CPAN separately - no reason it needs to be distributed
> with Git itself.
>
> See http://utsl.gen.nz/gitweb/?p=VCS-Git
Could you please put appropriate information on GitWiki
http://git.or.cz/gitwiki/InterfacesFrontendsAndTools
Perhaps it would be good time to start new section, Git Implementations,
and put egit (Java GIT library and Eclipse plugin) there too.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* [PATCH] gitweb: Make pickaxe search a feature
From: Jakub Narebski @ 2006-09-10 22:29 UTC (permalink / raw)
To: git
As pickaxe search (selected using undocumented 'pickaxe:' operator in
search query) is resource consuming, allow to turn it on/off using
feature meachanism. Turned on by default, for historical reasons.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The difference compared to the previous version is that it is turned
on by default.
gitweb/gitweb.perl | 33 +++++++++++++++++++++++++++++++--
1 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d89f709..53e3478 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -90,6 +90,11 @@ our %feature = (
'override' => 0,
# => [content-encoding, suffix, program]
'default' => ['x-gzip', 'gz', 'gzip']},
+
+ 'pickaxe' => {
+ 'sub' => \&feature_pickaxe,
+ 'override' => 0,
+ 'default' => [1]},
);
sub gitweb_check_feature {
@@ -143,6 +148,24 @@ sub feature_snapshot {
return ($ctype, $suffix, $command);
}
+# To enable system wide have in $GITWEB_CONFIG
+# $feature{'pickaxe'}{'default'} = [1];
+# To have project specific config enable override in $GITWEB_CONFIG
+# $feature{'pickaxe'}{'override'} = 1;
+# and in project config gitweb.pickaxe = 0|1;
+
+sub feature_pickaxe {
+ my ($val) = git_get_project_config('pickaxe', '--bool');
+
+ if ($val eq 'true') {
+ return (1);
+ } elsif ($val eq 'false') {
+ return (0);
+ }
+
+ return ($_[0]);
+}
+
# rename detection options for git-diff and git-diff-tree
# - default is '-M', with the cost proportional to
# (number of removed files) * (number of new files).
@@ -3128,8 +3151,7 @@ sub git_search {
if (!%co) {
die_error(undef, "Unknown commit object");
}
- # pickaxe may take all resources of your box and run for several minutes
- # with every query - so decide by yourself how public you make this feature :)
+
my $commit_search = 1;
my $author_search = 0;
my $committer_search = 0;
@@ -3141,6 +3163,13 @@ sub git_search {
} elsif ($searchtext =~ s/^pickaxe\\://i) {
$commit_search = 0;
$pickaxe_search = 1;
+
+ # pickaxe may take all resources of your box and run for several minutes
+ # with every query - so decide by yourself how public you make this feature
+ my ($have_pickaxe) = gitweb_check_feature('pickaxe');
+ if (!$have_pickaxe) {
+ die_error('403 Permission denied', "Permission denied");
+ }
}
git_header_html();
git_print_page_nav('','', $hash,$co{'tree'},$hash);
--
1.4.2
^ permalink raw reply related
* [PATCH] gitweb: Paginate history output
From: Jakub Narebski @ 2006-09-10 22:36 UTC (permalink / raw)
To: git
git_history output is now divided into pages, like git_shortlog,
git_tags and git_heads output. As whole git-rev-list output is now
read into array before writing anything, it allows for better
signaling of errors.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This can be seen as the part of series consolidating log-like
view (shortlog, log, history, perhaps tags and heads) generation.
gitweb/gitweb.perl | 64 ++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 53e3478..c3544dd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1933,12 +1933,15 @@ sub git_shortlog_body {
sub git_history_body {
# Warning: assumes constant type (blob or tree) during history
- my ($fd, $refs, $hash_base, $ftype, $extra) = @_;
+ my ($revlist, $from, $to, $refs, $hash_base, $ftype, $extra) = @_;
+
+ $from = 0 unless defined $from;
+ $to = $#{$revlist} unless (defined $to && $to <= $#{$revlist});
print "<table class=\"history\" cellspacing=\"0\">\n";
my $alternate = 0;
- while (my $line = <$fd>) {
- if ($line !~ m/^([0-9a-fA-F]{40})/) {
+ for (my $i = $from; $i <= $to; $i++) {
+ if ($revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {
next;
}
@@ -3114,29 +3117,70 @@ sub git_history {
if (!defined $hash_base) {
$hash_base = git_get_head_hash($project);
}
+ if (!defined $page) {
+ $page = 0;
+ }
my $ftype;
my %co = parse_commit($hash_base);
if (!%co) {
die_error(undef, "Unknown commit object");
}
+
my $refs = git_get_references();
- git_header_html();
- git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base);
- git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+ my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
+
if (!defined $hash && defined $file_name) {
$hash = git_get_hash_by_path($hash_base, $file_name);
}
if (defined $hash) {
$ftype = git_get_type($hash);
}
- git_print_page_path($file_name, $ftype, $hash_base);
open my $fd, "-|",
- git_cmd(), "rev-list", "--full-history", $hash_base, "--", $file_name;
+ git_cmd(), "rev-list", $limit, "--full-history", $hash_base, "--", $file_name
+ or die_error(undef, "Open git-rev-list-failed");
+ my @revlist = map { chomp; $_ } <$fd>;
+ close $fd
+ or die_error(undef, "Reading git-rev-list failed");
+
+ my $paging_nav = '';
+ if ($page > 0) {
+ $paging_nav .=
+ $cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
+ file_name=>$file_name)},
+ "first");
+ $paging_nav .= " ⋅ " .
+ $cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
+ file_name=>$file_name, page=>$page-1),
+ -accesskey => "p", -title => "Alt-p"}, "prev");
+ } else {
+ $paging_nav .= "first";
+ $paging_nav .= " ⋅ prev";
+ }
+ if ($#revlist >= (100 * ($page+1)-1)) {
+ $paging_nav .= " ⋅ " .
+ $cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
+ file_name=>$file_name, page=>$page+1),
+ -accesskey => "n", -title => "Alt-n"}, "next");
+ } else {
+ $paging_nav .= " ⋅ next";
+ }
+ my $next_link = '';
+ if ($#revlist >= (100 * ($page+1)-1)) {
+ $next_link =
+ $cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
+ file_name=>$file_name, page=>$page+1),
+ -title => "Alt-n"}, "next");
+ }
- git_history_body($fd, $refs, $hash_base, $ftype);
+ git_header_html();
+ git_print_page_nav('history','', $hash_base,$co{'tree'},$hash_base, $paging_nav);
+ git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+ git_print_page_path($file_name, $ftype, $hash_base);
+
+ git_history_body(\@revlist, ($page * 100), $#revlist,
+ $refs, $hash_base, $ftype, $next_link);
- close $fd;
git_footer_html();
}
--
1.4.2
^ permalink raw reply related
* Re: Change set based shallow clone
From: Paul Mackerras @ 2006-09-10 22:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux, jonsmirl, git
In-Reply-To: <Pine.LNX.4.64.0609101254590.27779@g5.osdl.org>
Linus Torvalds writes:
> So we may do a fork/exit in a millisecond, but on the other hand, we can
> generally open a file in a microsecond. So we're definitely talking about
> several orders of magnitude difference..
Unless the file isn't in memory, in which case it's probably around a
millisecond or more...
Do you think there is any way to speed up the cold-cache case for
git-ls-remote and git-rev-parse with thousands of heads and tags?
Paul.
^ permalink raw reply
* Re: Change set based shallow clone
From: Anand Kumria @ 2006-09-10 23:20 UTC (permalink / raw)
To: git
In-Reply-To: <9e4733910609071609o50e5dacm53323e023e90358f@mail.gmail.com>
On Thu, 07 Sep 2006 19:09:21 -0400, Jon Smirl wrote:
> On 9/7/06, Junio C Hamano <junkio@cox.net> wrote:
>> "Jon Smirl" <jonsmirl@gmail.com> writes:
>>
>> > Does an average user do these things? The shallow clone is there to
>> > address the casual user who gags at a five hour download to get an
>> > initial check out Mozilla when they want to make a five line change or
>> > just browse the source for a few minutes.
>> >...
>> > Maybe the answer is to build a shallow clone tool for casual use, and
>> > then if you try to run anything too complex on it git just tells you
>> > that you have to download the entire tree.
>>
>> For that kind of thing, "git-tar-tree --remote" would suffice I
>> would imagine. The five line change can be tracked locally by
>> creating an initial commit from the tar-tree extract; such a
>> casual user will not be pushing or asking to pull but sending in
>> patches to upstream, no?
>
> From my observation the casual user does something like this:
>
> get a shallow clone
This could basically be something which look at the remote HEAD and pulls
down a copy of that commit/tree (and associated objects), right?
> look at it for a while
> pull once a day to keep it up to date
Same again.
> decide to make some changes
> start a local branch
> commit changes on local branch
>
> push these changes to someone else for review
> maybe pull changes on the branch back from the other person
[...]
At what point, if any, do you envisage a casual user pulling down a full
copy of the repository?
Cheers,
Anand
^ permalink raw reply
* Re: Change set based shallow clone
From: Shawn Pearce @ 2006-09-11 0:03 UTC (permalink / raw)
To: Paul Mackerras
Cc: Jon Smirl, Linus Torvalds, linux@horizon.com, Git Mailing List
In-Reply-To: <17668.2019.732961.855446@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> wrote:
[snip]
> The bottom line is that I can speed up the startup for the hot-cache
> case quite a lot. The cold-cache case is going to take about 20-30
> seconds whatever I do unless Linus or Junio can come up with a way to
> pack the heads and tags. I could read the refs asynchronously but
> there will still be a long delay in git rev-parse if you give
> arguments such as --all.
I've been thinking about implementing ref storage within a Git tree
object. Just store the commit/tag/object IDs in a tree (or graph
of trees) with a mode of '0'. Anchor that under '.git/refs-tree'.
Any edit of a ref would "lock" .git/refs-tree, create a new tree
containing the update, then replace .git/refs-tree.
But it would put additional stress on the objects directory by
creating a lot of trees which would never get pulled into pack
files and thus would need to be pruned away on a regular basis.
It also would make parallel updates more difficult on the server
side as everyone would need to wait for the lock to .git/refs-tree
before they can change any ref; today users only need to wait for
the ref they are trying to change.
It also doesn't help looking up a ref quickly; although trees are
sorted they are variable length entries which forces the application
to read the entire tree to find its entry.
Given those three downsides I haven't put anything to code yet.
--
Shawn.
^ 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