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

Erik Joelsson erik.joelsson at oracle.com
Fri Jul 5 07:24:37 UTC 2013


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)

/Erik

On 2013-07-04 15:41, Rob McKenna wrote:
> 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 have a couple of questions about this:
>
> 1) I'm thinking of including a unit test which will run through the 
> various permutations across each platform making sure we fail when 
> expected.
>
> 2) Should we simply ignore the flag on Windows or should we throw the 
> same error if its provided?
>
> cc'ing Erik for a build review.
>
> http://cr.openjdk.java.net/~robm/5049299/webrev.04/
>
>     -Rob
>
>
> On 02/07/13 16:41, Alan Bateman wrote:
>> 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 don't show up in my webrev or hg diff, 
>>> but I see them in the jdk.patch generated by webrev. I have no idea 
>>> why this is. (I did use the JEXEC stuff as a template for the 
>>> JSPAWNHELPER code, but I don't recall altering the JEXEC stuff in 
>>> any way. It looks like its limited to indentation. Strange.)
>>>
>>> W.r.t. useFork, I'll discuss this with Alan once he has a spare 
>>> minute. I believe he may have had some concerns, but I'll let you 
>>> know how that goes either way.
>>>
>>> The only __APPLE__ should be to get the call to _NSGetEnviron(). 
>>> Everything else should be bsd.
>>>
>>> I've put a webrev at:
>>>
>>> http://cr.openjdk.java.net/~robm/5049299/webrev.03/ 
>>> <http://cr.openjdk.java.net/%7Erobm/5049299/webrev.03/>
>>>
>>> I hope this addresses the rest of your comments.
>>>
>>>     -Rob
>> Rob - this was great work but I don't think we brought it to a 
>> conclusion. Are you planning to re-base the patch and send out a new 
>> webrev?
>>
>> -Alan
>



More information about the core-libs-dev mailing list