X-Spam-Check-By: sourceware.org Message-ID: <4559D376.9030707@byu.net> Date: Tue, 14 Nov 2006 07:32:22 -0700 From: Eric Blake User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.8) Gecko/20061025 Thunderbird/1.5.0.8 Mnenhy/0.7.4.666 MIME-Version: 1.0 To: cygwin AT cygwin DOT com, grenie AT matapp DOT unimib DOT it Subject: Re: Excessive thrashing when popen()ing after a large malloc() References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner AT cygwin DOT com Mail-Followup-To: cygwin AT cygwin DOT com Delivered-To: mailing list cygwin AT cygwin DOT com -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 According to Loic Grenie on 11/14/2006 4:12 AM: > The subjects says it all: when a process has a large memory space, > a popen() triggers a long disk thrashing. The result can clearly be > seen iwth the allegated cygtharsh program (running with 1GB of memory, > change the size of the malloc to roughly half your memory size): Indeed - the fork/exec model of the current popen implementation is harsh on large memory spaces, compared to your proposed spawn model. While the idea may have merit, you have some work to do before a patch can be accepted. > > > to something similar to (warning: untested, needs a char *cmd; at the > beggining): Hint - popen is implemented in newlib, not cygwin, so you are posting to the wrong list. Unless you provide a 'diff -u' patch, it is very difficult to see your changes in context. And admitting that your changes are untested is not a good sign for getting it approved. > > > if (cmd = malloc(strlen(program)+64)) == NULL) { > (void)close(pdes[1]); > (void)close(pdes[2]); > free(cur); > return (NULL); > } > if (*type == 'r') { > if (pdes[1] != STDOUT_FILENO) > sprintf(cmd, "sh -c '%s' >&%d %d>&-", program, pdes[1], pdes[1]); > else > sprintf(cmd, "sh -c '%s'", program); This mishandles a program that contains embedded '. > if (pdes[0] != STDOUT_FILENO) > sprintf(cmd+strlen(cmd), " %d>&-", pdes[0]); Won't work if cmd already contains redirections. If you intend for your redirection to occur first, you will have to do some more work. > } > else { > if (pdes[0] != STDIN_FILENO) > sprintf(cmd, "sh -c '%s' <&%d %d>&- %d>&-", program, > pdes[0], pdes[0], pdes[1]); I'm not sure your redirections are correct here. > else > sprintf(cmd, "sh -c '%s' %d>&-", program, pdes[1]) > } > pid = spawnl(_P_NOWAIT, _PATH_BSHELL, "sh", "-c", cmd, NULL); Why are you going through two levels of sh? That seems like a waste to me; the whole idea of using spawn is to avoid a fork(), but when you invoke "sh" "-c" "sh -c 'cmd'", you are right back to a fork. True, the new invocation of sh uses less memory than the 1 GB process that invoked popen, so less thrashing will occur, but your whole approach seems fundamentally flawed if you are trying to use spawn to avoid a fork. - -- Life is short - so eat dessert first! Eric Blake ebb9 AT byu DOT net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFFWdN184KuGfSFAYARArdkAKCm4fzNrB0j3xv7X7eQNcoreCASswCeP3r3 N94x7ZElgUuI3Rw5Fw+X3j0= =EhF9 -----END PGP SIGNATURE----- -- Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple Problem reports: http://cygwin.com/problems.html Documentation: http://cygwin.com/docs.html FAQ: http://cygwin.com/faq/