Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-08-12 Thread Alan Bateman
On 10/08/2013 17:16, Rob McKenna wrote: Thanks for the build review Erik, Hi Alan, Thanks for the review. I'm hoping this webrev has dealt with your comments. http://cr.openjdk.java.net/~robm/5049299/webrev.06/ A couple of notes: -

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-08-10 Thread Rob McKenna
Thanks for the build review Erik, Hi Alan, Thanks for the review. I'm hoping this webrev has dealt with your comments. http://cr.openjdk.java.net/~robm/5049299/webrev.06/ A couple of notes: - The jexec comments in CompileLauncher refer

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-28 Thread Alan Bateman
On 25/07/2013 09:17, Rob McKenna wrote: Thanks a lot Erik, I've added the dependency to the makefile here: http://cr.openjdk.java.net/~robm/5049299/webrev.05/ Is it ok between the ifeq block? Alan, I've altered the launchMechanism cod

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-25 Thread Rob McKenna
Thanks a lot Erik, I've added the dependency to the makefile here: http://cr.openjdk.java.net/~robm/5049299/webrev.05/ Is it ok between the ifeq block? Alan, I've altered the launchMechanism code to use valueOf (and lower case names)

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-05 Thread Erik Joelsson
Build changes are looking pretty good. Just one thing that I would like to add. Since the executable jspawnhelper is linking in an object file from libjava, it would be good to declare that dependency. See unpack200 for a similar situation. $(BUILD_JSPAWNHELPER): $(LINK_JSPAWNHELPER_OBJECTS)

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-04 Thread Alan Bateman
On 04/07/2013 14:41, Rob McKenna wrote: Hi Alan, I've just uploaded the latest changes which rebase the fix to the current repo. Thank you (as you know I've been wanting us to move to posix_spawn on Mac and Solaris for a long time). (required post 8008118) I've also swapped out useFork for

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-04 Thread Rob McKenna
Hi Alan, I've just uploaded the latest changes which rebase the fix to the current repo. (required post 8008118) I've also swapped out useFork for jdk.lang.Process.launchMechanism. (it affords us the flexibility to enable or disable other mechanisms on the various platforms at a later date) I

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2013-07-02 Thread Alan Bateman
On 23/12/2012 01:19, Rob McKenna wrote: Hi Martin, thanks a lot for this. I've renamed LINKFLAG to the more appropriate (and common) ARCHFLAG. It seems to pop up all around our source but if build-dev know of a better (or canonical) way of doing it I'll take it! The BUILD_JEXEC differences d

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-12-22 Thread Rob McKenna
Hi Martin, thanks a lot for this. I've renamed LINKFLAG to the more appropriate (and common) ARCHFLAG. It seems to pop up all around our source but if build-dev know of a better (or canonical) way of doing it I'll take it! The BUILD_JEXEC differences don't show up in my webrev or hg diff, but

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-12-20 Thread Erik Joelsson
Hello, Nice to see a good attempt at getting the new build in order. While this looks like it works, I have a few comments: In makefiles/CompileLaunchers.gmk * OPENJDK_TARGET_CPU_LIBDIR is empty on macosx, so you can safely use the same definition of BUILD_JSPAWNHELPER_DST_DIR for all platfo

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-12-19 Thread Martin Buchholz
+res = readFully (fdin, &magic, sizeof(magic)); +if (res != 4 || magic != magicNumber()) { s/4/sizeof(magic)/ --- +extern int errno; Delete. --- +#define ALLOC(X,Y) { \ +void *mptr; \ +mptr = malloc (Y); \ +if (mptr == 0) { \ +error (fdout, ERR_MALLOC); \ +} \

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-12-19 Thread Martin Buchholz
Thanks for this. I agree with the strategy. I'm hoping build folks and macosx folks also take a look at this hairy change. +LINKFLAG = +ifeq ($(ARCH_DATA_MODEL), 64) +LINKFLAG = -m64 +endif It looks wrong to be specifying toolchain-specific flags here. Also, -m64 is logically a compiler flag,

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-12-19 Thread Rob McKenna
Hi folks, Thanks for the feedback so far. I've uploaded a new webrev to: http://cr.openjdk.java.net/~robm/5049299/webrev.02/ I've made the following headline changes: - Initial effort to get this stuff into the new build-infra. Hoping

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-29 Thread Rob McKenna
Hi David, On 30/11/12 02:31, David Holmes wrote: Hi Rob, This is only a superficial scan. The changes in java/java/makefile look pretty horrible. What are all those -R entries? Library search paths. Currently jprochelper is linked to libjava. I'm hoping to either cut their number (by alterin

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-29 Thread David Holmes
Hi Rob, This is only a superficial scan. The changes in java/java/makefile look pretty horrible. What are all those -R entries? We will need equivalent changes for the new build system before this is pushed. Is the spawn use BSD specific (as per UnixProcess.java.BSD) or Apple specific (as

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-27 Thread Martin Buchholz
$javahome/lib/jexec is a precedent for implementation detail executables not found in $javahome/bin

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-27 Thread Rob McKenna
I'll put a test together for this before sending out another review. I was sure I had some sort of issue with this in testing before, but perhaps it was IPC related. -Rob On 27/11/12 17:48, Alan Bateman wrote: On 27/11/2012 17:45, Martin Buchholz wrote: On Tue, Nov 27, 2012 at 3:06 AM,

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-27 Thread Alan Bateman
On 27/11/2012 17:45, Martin Buchholz wrote: On Tue, Nov 27, 2012 at 3:06 AM, Alan Bateman > wrote: On 27/11/2012 06:47, Martin Buchholz wrote: : On Solaris bi-arch I think you need only one jprochelper, not two, since a 32-bit helper can exec

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-27 Thread Martin Buchholz
On Tue, Nov 27, 2012 at 3:06 AM, Alan Bateman wrote: > On 27/11/2012 06:47, Martin Buchholz wrote: > > : > > On Solaris bi-arch I think you need only one jprochelper, not two, since > a 32-bit helper can exec a 64-bit subprocess. > > This is a good point, it needs to know if the target program

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-27 Thread Alan Bateman
On 27/11/2012 06:47, Martin Buchholz wrote: : On Solaris bi-arch I think you need only one jprochelper, not two, since a 32-bit helper can exec a 64-bit subprocess. This is a good point, it needs to know if the target program is 32-bit or 64-bit and chose the appropriate trampoline/helper.

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-26 Thread Martin Buchholz
Rob, Thanks for taking on this big scary change. Our experience having run with the vfork based implementation on Linux has been very positive. This addresses a real need that is shared by all big processes that desire offspring. You might want to look through my comments from the last round of

Re: Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-23 Thread Alan Bateman
On 22/11/2012 21:27, Rob McKenna wrote: Hi folks, Looking for a review for the webrev below, which also resolves: 7175692: (process) Process.exec should use posix_spawn [macosx] For additional context and a brief description it would be well worth looking at the following thread started by Mi

Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

2012-11-22 Thread Rob McKenna
Hi folks, Looking for a review for the webrev below, which also resolves: 7175692: (process) Process.exec should use posix_spawn [macosx] For additional context and a brief description it would be well worth looking at the following thread started by Michael McMahon, who did the brunt of the