UNIXProcess improvements

[Martin Buchholz]

Thanks for the careful review.

On Wed, Apr 21, 2010 at 17:47, David Schlosnagle <schlosna at gmail.com> wrote:
> Martin,
>
> In src/solaris/classes/java/lang/UNIXProcess.java.linux, are lines
> 177, 181, and 185 needed? If the assignment of these streams were in
> the constructor, they could be final.

Fixed.

Yes, the streams really should be final,
but it's not worth the boilerplate handsprings to make them so.

-    private OutputStream stdin_stream;
-    private InputStream  stdout_stream;
-    private InputStream  stderr_stream;
+    private /* final */ OutputStream stdin;
+    private /* final */ InputStream  stdout;
+    private /* final */ InputStream  stderr;


>  176     synchronized void processExited(int exitcode) {
>  177         stdout = this.stdout;
>  178         if (stdout instanceof ProcessPipeInputStream)
>  179             ((ProcessPipeInputStream) stdout).processExited();
>  180
>  181         stderr = this.stderr;
>  182         if (stderr instanceof ProcessPipeInputStream)
>  183             ((ProcessPipeInputStream) stderr).processExited();
>  184
>  185         stdin = this.stdin;
>  186         if (stdin instanceof ProcessPipeOutputStream)
>  187             ((ProcessPipeOutputStream) stdin).processExited();
>  188
>  189         this.exitcode = exitcode;
>  190         hasExited = true;
>  191         notifyAll();
>  192     }
>
> Minor nit, the java.io.ByteArrayOutputStream import is not used.

Fixed.  (do you have a tool to detect extra imports?)

Thanks,

Martin

> Thanks,
> Dave
>
> On Wed, Apr 21, 2010 at 7:15 PM, Martin Buchholz <martinrb at google.com> wrote:
>>
>> I now have the second part of my planned improvements to
>> Linux process handling, and is now a serious proposal.
>> for Michael and others to review.
>>
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/UNIXProcess/
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/UNIXProcess2/
>>
>> This is a huge performance and reliability improvement for
>> java programs that create many processes.
>> The program below (ManyProcesses) runs 4 times faster on my machine.
>>
>> This doesn't try to address Solaris.
>>
>> Martin
>>
>> import java.util.*;
>> import java.util.concurrent.atomic.*;
>>
>> public class ManyProcesses {
>>    public static void main(String[] args) throws Throwable {
>>        final int threadCount = 2;
>>        final int processCount = 2000;
>>        final AtomicLong count = new AtomicLong(0);
>>        Runnable r = new Runnable() {
>>          public void run() {
>>            try {
>>              for (int i = 0; i < processCount; i++) {
>>                Process p = new ProcessBuilder(new String[] {
>> "/bin/true" }).start();
>>                //if (p.waitFor() != 0) throw new Error();
>> //                 p.getInputStream().close();
>> //                 p.getOutputStream().close();
>> //                 p.getErrorStream().close();
>>                //count.getAndIncrement();
>>              }
>>            } catch (Throwable t) {
>>              throw new Error(t);
>>            }}};
>>        List<Thread> threads = new ArrayList<Thread>();
>>        for (int i = 0; i < threadCount; i++) {
>>          threads.add(new Thread(r));
>>        }
>>
>>        for (Thread thread : threads) thread.start();
>>        //Thread.sleep(1000 * 10);
>>        //System.out.println(count.get());
>>        for (Thread thread : threads) thread.join();
>>        System.out.println(new Thread().getId());
>>    }
>> }
>>
>> On Tue, Apr 20, 2010 at 08:58, Michael McMahon <Michael.McMahon at sun.com> wrote:
>> > Martin,
>> >
>> > Thanks for the answers. The changes look fine to me.
>> >
>> > - Michael.
>> >
>> > Martin Buchholz wrote:
>> >>
>> >> On Mon, Apr 19, 2010 at 09:04, Michael McMahon <Michael.McMahon at sun.com>
>> >> wrote:
>> >>
>> >>>
>> >>> Martin Buchholz wrote:
>> >>>
>> >>>>
>> >>>> On Fri, Apr 16, 2010 at 09:18, Mark Reinhold <mr at sun.com> wrote:
>> >>>>
>> >>>>
>> >>>>
>> >>>>>
>> >>>>> For now I suggest leaving old @author tags as-is.
>> >>>>>
>> >>>>>
>> >>>>
>> >>>> OK, done.
>> >>>>
>> >>>> Version 0.2 of the webrev is published.
>> >>>>
>> >>>> Martin
>> >>>>
>> >>>>
>> >>>
>> >>> Martin,
>> >>>
>> >>> From what I can see, you've cleaned up the code and the functional
>> >>> changes
>> >>> are the use of a thread pool, and an explicit (8 k sized) stack.
>> >>>
>> >>
>> >> Exceptions thrown in the "process reaper" thread,
>> >> which were not IOExceptions,
>> >> were not being caught, and would cause the user thread to hang.
>> >>
>> >>
>> >>>
>> >>> Also, the threads created now belong to the root thread group rather than
>> >>> the application's thread group.
>> >>>
>> >>
>> >> Well, they have to belong to some thread group,
>> >> and they get reused, so in general the thread group will have
>> >> no relation to the thread group of the user thread,
>> >> so may as well use the root thread group.
>> >> I stole this technique from elsewhere in the JDK.
>> >>
>> >>
>> >>>
>> >>> Is this so you can handle uncaught
>> >>> exceptions
>> >>> as you mentioned before, and if so, I guess some other change is coming
>> >>> to
>> >>> complete
>> >>> this work. Is that right?
>> >>>
>> >>
>> >> Yes.  This change by itself is a clear win,
>> >> except that because it is more memory efficient,
>> >> GC is less likely to get called,
>> >> which means file descriptors of defunct processes
>> >> are less likely to get cleaned up in the face of
>> >> lazy user code, which means it is more subject
>> >> to file descriptor exhaustion problems.
>> >> Which I would like to fix.
>> >>
>> >> Martin
>> >>
>> >>
>> >>>
>> >>> Thanks,
>> >>> Michael
>> >>>
>> >>>
>> >>>
>> >
>> >
>