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

Martin Buchholz martinrb at google.com
Tue Nov 27 06:47:19 UTC 2012

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 review, back when I had more brain cells.

I agree giving people the choice to use the algorithm using a system property is a good one.

The strategy of using posix_spawn of a small helper process seems good (more portable than vfork or clone). If it works well universally, you might even consider making it the default on Linux (although I worry about

clone is not known to work reliably (I tried and failed). I would leave it #ifdef'ed out by default.

don't put the helper program in JAVAHOME/bin because users should never invoke it directly - it shouldn't be on anyone's PATH.

Not sure why so many dirs in HELPERLDFLAGS. I would think that the prochelper program would be a tiny C programs with no need to pull in any other jdk libraries.

On Solaris bi-arch I think you need only one jprochelper, not two, since a 32-bit helper can exec a 64-bit subprocess.

+#if defined(solaris) || defined(APPLE) +#include <spawn.h> +#endif

I think you want to autoconfiscate something like HAVE_POSIX_SPAWN instead.

+#ifdef _CS_PATH

I would separate out smaller less risky improvements like use of _CS_PATH into a separate change.

Martin

On Fri, Nov 23, 2012 at 3:56 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

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 posixspawn [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 work for this fix: http://mail.openjdk.java.net/pipermail/core-libs-dev/2009- May/thread.html#1644<http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-May/thread.html#1644> Basically the fix aims to swap fork for posixspawn as the default process launch mechanism on Solaris and Mac OSX in order to avoid swap exhaustion issues encountered with fork()/exec(). It also offers a flag (java.lang.useFork) to allow a user to revert to the old behaviour. I'm having trouble seeing the wood for the trees at this point so I'm anticipating plenty of feedback. In particular I'd appreciate some discussion on: - The binary launcher name & property name may need some work. A more general property ("java.lang.launchMechanism") to allow a user to specify a particular call was mooted too. It may be more future proof and I'm completely open to that. (e.g. launchMechanism=spawn|fork|**vfork|clone - we would obviously ignore inapplicable values on a per-platform basis) - I'd like a more robust way of checking that someone isn't trying to use jprochelper outside of the context for which it is meant. - The decision around which call to use getting moved to the java level and away from the native preprocessor. The webrev is at: http://cr.openjdk.java.net/**robm/5049299/webrev.01/<http://cr.openjdk.java.net/robm/5049299/webrev.01/><_ _http://cr.openjdk.java.net/%**7Erobm/5049299/webrev.01/<http://cr.openjdk.java.net/%7Erobm/5049299/webrev.01/> > It's great to get this one moving again, we should have helped Michael more to get this over the line on the first outing. This one will require very careful review, I don't have cycles this week, I hope others do. For now I think that naming the trampoline jprochelper or jspawnhelper is okay. To make it more robust then I'd probably prepend a magic number or some token. Also I'd probably fstat stdin and uses SFIFO to check the mode. As the property is implementation specific then I think something like jdk.lang.process.useFork is a better start. It would be nice not to require it although I agree we have to walk carefully as this area has a tendency to bite back. I don't think you need to make it any more configurable than that. If this is successful then I think we should look at updating the hotspot code too as it has the same issue with VM options such as OnError. -Alan.

More information about the core-libs-dev mailing list