{"thread":{"id":"14602","subject":"[PATCH] git daemon: avoid waking up too often","startedAt":"2008-07-22T21:52:13Z","lastAt":"2008-07-23T19:39:56Z","messageCount":6,"participants":["Johannes Schindelin","Johannes Sixt","Avery Pennarun"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"84385","messageId":"alpine.DEB.1.00.0807222251570.8986@racer","threadId":"14602","inReplyTo":null,"subject":"[PATCH] git daemon: avoid waking up too often","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-22T21:52:13Z","receivedAt":"2008-07-22T21:52:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nTo avoid waking up unnecessarily, a pipe is set up that is only ever\nwritten to by child_handler(), when a child disconnects, as suggested\nper Junio.\n\nThis avoids waking up the main process every second to see if a child\nwas disconnected.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tI have this in production ever since I posted it the first time,\n\twithout problems.\n\n\tOh, and it removes more lines than it introduces ;-)\n\n daemon.c |   24 +++++++++++-------------\n 1 files changed, 11 insertions(+), 13 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 7df41a6..850f6f9 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -16,6 +16,7 @@\n static int log_syslog;\n static int verbose;\n static int reuseaddr;\n+static int child_handler_pipe[2];\n \n static const char daemon_usage[] =\n \"git daemon [--verbose] [--syslog] [--export-all]\\n\"\n@@ -788,6 +789,7 @@ static void child_handler(int signo)\n \t\t\t\tpid = -pid;\n \t\t\tdead_child[reaped % MAX_CHILDREN] = pid;\n \t\t\tchildren_reaped = reaped + 1;\n+\t\t\twrite(child_handler_pipe[1], &status, 1);\n \t\t\tcontinue;\n \t\t}\n \t\tbreak;\n@@ -933,12 +935,17 @@ static int service_loop(int socknum, int *socklist)\n \tstruct pollfd *pfd;\n \tint i;\n \n-\tpfd = xcalloc(socknum, sizeof(struct pollfd));\n+\tif (pipe(child_handler_pipe) < 0)\n+\t\tdie (\"Could not set up pipe for child handler\");\n+\n+\tpfd = xcalloc(socknum + 1, sizeof(struct pollfd));\n \n \tfor (i = 0; i < socknum; i++) {\n \t\tpfd[i].fd = socklist[i];\n \t\tpfd[i].events = POLLIN;\n \t}\n+\tpfd[socknum].fd = child_handler_pipe[0];\n+\tpfd[socknum].events = POLLIN;\n \n \tsignal(SIGCHLD, child_handler);\n \n@@ -946,16 +953,7 @@ static int service_loop(int socknum, int *socklist)\n \t\tint i;\n \t\tint timeout;\n \n-\t\t/*\n-\t\t * This 1-sec timeout could lead to idly looping but it is\n-\t\t * here so that children culled in child_handler() are reported\n-\t\t * without too much delay.  We could probably set up a pipe\n-\t\t * to ourselves that we poll, and write to the fd from child_handler()\n-\t\t * to wake us up (and consume it when the poll() returns...\n-\t\t */\n-\t\ttimeout = (children_spawned != children_deleted) ? 1000 : -1;\n-\t\ti = poll(pfd, socknum, timeout);\n-\t\tif (i < 0) {\n+\t\tif (poll(pfd, socknum + 1, -1) < 0) {\n \t\t\tif (errno != EINTR) {\n \t\t\t\terror(\"poll failed, resuming: %s\",\n \t\t\t\t      strerror(errno));\n@@ -963,9 +961,9 @@ static int service_loop(int socknum, int *socklist)\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n-\t\tif (i == 0) {\n+\t\tif (pfd[socknum].revents & POLLIN) {\n+\t\t\tread(child_handler_pipe[0], &i, 1);\n \t\t\tcheck_dead_children();\n-\t\t\tcontinue;\n \t\t}\n \n \t\tfor (i = 0; i < socknum; i++) {\n-- \n1.6.0.rc0.22.gf2096d.dirty\n"},{"id":"84387","messageId":"alpine.DEB.1.00.0807222302440.8986@racer","threadId":"14602","inReplyTo":"alpine.DEB.1.00.0807222251570.8986@racer","subject":"[PATCH v2] git daemon: avoid waking up too often","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-22T22:03:01Z","receivedAt":"2008-07-22T22:03:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nTo avoid waking up unnecessarily, a pipe is set up that is only ever\nwritten to by child_handler(), when a child disconnects, as suggested\nper Junio.\n\nThis avoids waking up the main process every second to see if a child\nwas disconnected.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tDarn.  Forgot to commit after fixing the compiler warning:\n\tThe now-unused variable timeout is gone.\n\n\tThat is the only difference to the version I have running in\n\tproduction ;-)\n\n daemon.c |   25 +++++++++++--------------\n 1 files changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 7df41a6..4540e8d 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -16,6 +16,7 @@\n static int log_syslog;\n static int verbose;\n static int reuseaddr;\n+static int child_handler_pipe[2];\n \n static const char daemon_usage[] =\n \"git daemon [--verbose] [--syslog] [--export-all]\\n\"\n@@ -788,6 +789,7 @@ static void child_handler(int signo)\n \t\t\t\tpid = -pid;\n \t\t\tdead_child[reaped % MAX_CHILDREN] = pid;\n \t\t\tchildren_reaped = reaped + 1;\n+\t\t\twrite(child_handler_pipe[1], &status, 1);\n \t\t\tcontinue;\n \t\t}\n \t\tbreak;\n@@ -933,29 +935,24 @@ static int service_loop(int socknum, int *socklist)\n \tstruct pollfd *pfd;\n \tint i;\n \n-\tpfd = xcalloc(socknum, sizeof(struct pollfd));\n+\tif (pipe(child_handler_pipe) < 0)\n+\t\tdie (\"Could not set up pipe for child handler\");\n+\n+\tpfd = xcalloc(socknum + 1, sizeof(struct pollfd));\n \n \tfor (i = 0; i < socknum; i++) {\n \t\tpfd[i].fd = socklist[i];\n \t\tpfd[i].events = POLLIN;\n \t}\n+\tpfd[socknum].fd = child_handler_pipe[0];\n+\tpfd[socknum].events = POLLIN;\n \n \tsignal(SIGCHLD, child_handler);\n \n \tfor (;;) {\n \t\tint i;\n-\t\tint timeout;\n \n-\t\t/*\n-\t\t * This 1-sec timeout could lead to idly looping but it is\n-\t\t * here so that children culled in child_handler() are reported\n-\t\t * without too much delay.  We could probably set up a pipe\n-\t\t * to ourselves that we poll, and write to the fd from child_handler()\n-\t\t * to wake us up (and consume it when the poll() returns...\n-\t\t */\n-\t\ttimeout = (children_spawned != children_deleted) ? 1000 : -1;\n-\t\ti = poll(pfd, socknum, timeout);\n-\t\tif (i < 0) {\n+\t\tif (poll(pfd, socknum + 1, -1) < 0) {\n \t\t\tif (errno != EINTR) {\n \t\t\t\terror(\"poll failed, resuming: %s\",\n \t\t\t\t      strerror(errno));\n@@ -963,9 +960,9 @@ static int service_loop(int socknum, int *socklist)\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n-\t\tif (i == 0) {\n+\t\tif (pfd[socknum].revents & POLLIN) {\n+\t\t\tread(child_handler_pipe[0], &i, 1);\n \t\t\tcheck_dead_children();\n-\t\t\tcontinue;\n \t\t}\n \n \t\tfor (i = 0; i < socknum; i++) {\n-- \n1.6.0.rc0.22.gf2096d.dirty\n"},{"id":"84440","messageId":"4886D503.7030106@viscovery.net","threadId":"14602","inReplyTo":"alpine.DEB.1.00.0807222302440.8986@racer","subject":"Re: [PATCH v2] git daemon: avoid waking up too often","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-07-23T06:51:47Z","receivedAt":"2008-07-23T06:51:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> To avoid waking up unnecessarily, a pipe is set up that is only ever\n> written to by child_handler(), when a child disconnects, as suggested\n> per Junio.\n> \n> This avoids waking up the main process every second to see if a child\n> was disconnected.\n\nThis makes porting this beast to Windows practically impossible because we\ncannot have a poll() implementation that waits both on a listening socket\nand a pipe. :-(\n\n-- Hannes\n"},{"id":"84451","messageId":"alpine.DEB.1.00.0807231112310.2830@eeepc-johanness","threadId":"14602","inReplyTo":"4886D503.7030106@viscovery.net","subject":"Re: [PATCH v2] git daemon: avoid waking up too often","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-23T09:17:00Z","receivedAt":"2008-07-23T09:17:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Jul 2008, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > To avoid waking up unnecessarily, a pipe is set up that is only ever\n> > written to by child_handler(), when a child disconnects, as suggested\n> > per Junio.\n> > \n> > This avoids waking up the main process every second to see if a child\n> > was disconnected.\n> \n> This makes porting this beast to Windows practically impossible because we\n> cannot have a poll() implementation that waits both on a listening socket\n> and a pipe. :-(\n\nUmm.  We also do not have signal handlers, do we?  AFAICT you wanted to \nsimulate _this_ program's fork() with a start_async(), right?  All you \nhave to do is to refactor the code with the pipe to do the update \n(properly guarded by a mutex, which is called \"CriticalSection\" in Windows \nspeak, since it is impossible for Microsoft to accept common conventions).\n\nThe biggest part to get this beast running on Windows is to move it to the \nstart_async() method, not above-mentioned refactoring.\n\nCiao,\nDscho\n"},{"id":"84588","messageId":"32541b130807231226m73fcc874ub72f69adeb5d26c3@mail.gmail.com","threadId":"14602","inReplyTo":"4886D503.7030106@viscovery.net","subject":"Re: [PATCH v2] git daemon: avoid waking up too often","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2008-07-23T19:26:32Z","receivedAt":"2008-07-23T19:26:32Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On 7/23/08, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> This makes porting this beast to Windows practically impossible because we\n>  cannot have a poll() implementation that waits both on a listening socket\n>  and a pipe. :-(\n\nI have worked around such problems in the past by having a thread\nwhose job it is to read from the pipe and simply write it to a socket.\n The trick works for \"console\" objects, too, which in win32 are even\nless agreeable than pipes.\n\n(In case your life wasn't disgusting enough already today :))\n\nAlternatively, you could use something like socketpair() instead of a\npipe for this purpose.  Naturally, Win32 helps you out here by somehow\nforgetting to include socketpair() in winsock, although it's sort of\neasy to emulate.\n\nHave fun,\n\nAvery\n"},{"id":"84593","messageId":"alpine.DEB.1.00.0807232038310.8986@racer","threadId":"14602","inReplyTo":"32541b130807231226m73fcc874ub72f69adeb5d26c3@mail.gmail.com","subject":"Re: [PATCH v2] git daemon: avoid waking up too often","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-23T19:39:56Z","receivedAt":"2008-07-23T19:39:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Jul 2008, Avery Pennarun wrote:\n\n> On 7/23/08, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> > This makes porting this beast to Windows practically impossible \n> > because we cannot have a poll() implementation that waits both on a \n> > listening socket and a pipe. :-(\n> \n> I have worked around such problems in the past by having a thread whose \n> job it is to read from the pipe and simply write it to a socket. The \n> trick works for \"console\" objects, too, which in win32 are even less \n> agreeable than pipes.\n> \n> (In case your life wasn't disgusting enough already today :))\n\nThanks.  I thought the code I looked at today was ugly, but you convinced \nme that there is no limit.\n\n> Alternatively, you could use something like socketpair() instead of a \n> pipe for this purpose.  Naturally, Win32 helps you out here by somehow \n> forgetting to include socketpair() in winsock, although it's sort of \n> easy to emulate.\n\nOr alternatively, you could read my response to Hannes where I explain \nthat the pipe() is not even needed with threaded start_async() (as opposed \nto fork()ed one).\n\nCiao,\nDscho\n"}]}