Long computation causes ExecutorService to stop assigning new work

I am optimizing PNG files by spawning 5 pngout.exe processes to work on a directory of PNG files. Since pngout is single-threaded, this results in a large speedup. Some images take a long time to optimize, upwards of 30 seconds, while the norm is <5 seconds. The problem:

• File 1 is large, 2-5 are small, total of 50 files but details of rest irrelevant.
• First five pngout processes spawn normally and start working
• 2-5 exit within 10 seconds
• 1 takes 45 seconds
• No new pngout processes are spawned during this, despite having four threads free
• Upon completion of 1, another five processes are spawned.

Code:

private final ExecutorService pool = Executors.newFixedThreadPool(5);

    /* ^ instance var, below is in method */

    CompletionService<Boolean> comp = new ExecutorCompletionService<Boolean>(pool);
    List<Callable<Boolean>> tasks = new ArrayList<Callable<Boolean>>();
    for (int i = 0; i < files.length; i++) {
        File infile = files[i];
        File outfile = new File(outdir, infile.getName());
        tasks.add(new CrushTask(crusher, infile, outfile));
    }
    for (Callable<Boolean> t : tasks)
        comp.submit(t);
    for (int i = 0; i < files.length; i++) {
        try {
            boolean res = comp.take().get();
            System.out.println(res);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

All files are optimized properly, that part of the code works. The problem is that by waiting on large images, the entire process is massively slowed down. I get only a 40% improvement over single-threaded time.

What am I doing wrong?

edit: Fixed the issue, using some really ugly code. The problem is that to get the exit value of the processes I was spawning (to know when they are finished and if they succeeded) I was reading their stdout to nothing, since calling waitFor would hang forever. However, apparently using InputStreams makes threads choke.

So to get the exit value of the processes, instead of using this:

private static int discardStdOut(Process proc) throws IOException {
    final InputStream is = proc.getInputStream();
    try {
        while (is.read() != -1)
            continue;
        return proc.exitValue();
    } finally {
        close(is);
    }
}

I am using this gross code:

private static int discardStdOut(Process proc) {
    int ret = -1;
开发者_StackOverflow社区    while (true) {
        try {
            ret = proc.exitValue();
            break;
        } catch (IllegalThreadStateException e) {
            try {
                Thread.sleep(100);
            } catch (InterruptedException e2) {
                e2.printStackTrace();
            }
        }
    }
    return ret;
}

It's gross but now the system works fine and always has 5 processes running.

late edit: StreamGobbler from here is probably more proper.

---

You are getting Thread starvation. You need to do a sleep or IO for java to manage threading properly. Its not the JVMs fault operating system threading is broken.