Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I need to test my new functionality by sending about 100 requests in one time.

public class MyTest {
    // ...

    @Test
    public void test() throws Exception {
        Runnable[] runners = new Runnable[size];
        Thread[] threads = new Thread[size];

        for(int i=0; i<threads.length; i++) {
            runners[i] = new RequestThread(i);
        }

        for(int i=0; i<threads.length; i++) {
            threads[i] = new Thread(runners[i]);
        }

        for(int i=0; i<threads.length; i++) {
            threads[i].start();
        }

        while (!threadsAreDead(threads)) {}
    }

    private boolean threadsAreDead(Thread[] threads) {
        for(int i=0; i<threads.length; i++) {
            if (threads[i].isAlive()) return false;
        }       

        return true;
    }
}


class RequestThread implements Runnable {    
    public RequestThread(int id) {
        this.id = id;
    }

    @Override
    public void run() {
        try {
            b2b.execute(request);
        } catch (B2BException e) {
            // ...
        }
    }

}

Have you any idea how to optimize above code to send requests more 'in one time'?

share|improve this question
add comment

1 Answer 1

up vote 8 down vote accepted

Your test case looks relatively comprehensive in the sense that, yes, it fires off the 100 concurrent threads, but, you ask "how to make it more 'at the same time' ?"

There are two aspects I can suggest:

  • Use an ExecutorService
  • Use a CountDownLatch

ExecutorService

One of the novelties about the ExecutorService system is the concept of the Future, and it's foundation, the Callable class. The Callable interface is similar to the Runnable, except it can return a value, and throw an exception. These are two important differences.

The exception thrown in a Callable is trapped, and then re-thrown when the Future's get() method is called. This means that exceptions in remote threads can be collected in the calling thread.

In your case, throwing the exception is the most significant.

Consider the following code that sets up your threads appropriately:

@Test
public void test() throws Exception {
    ExecutorService threads = Executors.newFixedThreadPool(size);
    List<Callable<Boolean>> torun = new ArrayList<>(size);
    for (int i = 0; i < size; i++) {
        torun.add(new Callable<Integer>() {
            public Boolean call() throws B2BException {
                b2b.execute(request);
                return Boolean.TRUE;
            }
        });
    }

    // all tasks executed in different threads, at 'once'.
    List<Future<Boolean>> futures = threads.invokeAll(torun);

    // no more need for the threadpool
    threads.shutdown();
    // check the results of the tasks...throwing the first exception, if any.
    for (Future<Boolean> fut : futures) {
        fut.get();
    }

    //check the threadpool is now in fact complete
    if (!threads.isShutDown()) {
        // something went wrong... our accounting is off...
    }

}

OK, that is a better way of setting up the threads, and gets the exceptions to be thrown inside the test thread, rather than inside the remote threads.

CountDownLatch

If you are concerned that the threads should all be doing things more 'at the same time', and not worrying about start up time, etc. then consider using a count-down latch.

This device is set with a counter, and, when triggered, it will count down by one, and then lock, until all the counts are counted down.

You can use it in the above example code like:

@Test
public void test() throws Exception {
    ExecutorService threads = Executors.newFixedThreadPool(size);
    List<Callable<Boolean>> torun = new ArrayList<>(size);
    final CountDownLatch countdown = new CountDownLatch(size);
    for (int i = 0; i < size; i++) {
        toron.add(new Callable<Integer>() {
            public Boolean call() throws B2BException, InterruptedException {
                countdown.countDown();
                countdown.await();
                b2b.execute(request);
                return Boolean.TRUE;
            }
        });
    }

    // all tasks executed in different threads, at 'once'.
    List<Future<Boolean>> futures = threads.invokeAll(torun);

    // no more need for the threadpool
    threads.shutdown();
    // check the results of the tasks...throwing the first exception, if any.
    for (Future<Boolean> fut : futures) {
        fut.get();
    }

    //check the threadpool is now in fact complete
    if (!threads.isShutDown()) {
        // something went wrong... our accounting is off...
    }

}

The above system will set up all the threads, so they are all 'runnable', and then only when every thread is established and ready, will the locks be released.

You will still not get complete concurrent use, but it will/may be better having more threads start 'at once'.

Other....

You should consider using a custom ThreadFactory for the thread pool, like I suggested in this answer.

share|improve this answer
    
+1. List<Callable<Boolean>> could be List<Callable<Void>>. (new Callable<Integer>() looks inconsistent.) Check CyclicBarrier too, it was designed for these kind of problems. –  palacsint Mar 2 at 20:04
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.