Why blocking instead of looping?

Question

What are some reasons why writing the following piece of code is considered bad practice?

  while (someList.isEmpty()) {
    try {
      Thread.currentThread().sleep(100);
    }
    catch (Exception e) {}
  }
  // Do something to the list as soon as some thread adds an element to it.

To me, picking an arbitrary value to sleep is not good practice, and I would use a BlockingQueue in this situation, but I'd like to know if there is more than one reason why one shouldn't write such code.

Answer 1

It imposes an average delay of 50 milliseconds before the event is acted on, and it wakes up 10 times a second when there's no event to handle. If neither of those things particularly matter, then it's just inelegant.

Answer 2

There are many reasons not to do this. First, as you've noted, this means that there may be a large delay between the time that an event occurs that the thread should respond to and the actual response time, since the thread may be sleeping. Second, since any system has only so many different processors, if you have to keep kicking important threads off of a processor so that they can tell the thread to go to sleep another time, you decrease the total amount of useful work done by the system and increase the power usage of the system (which matters in systems like phones or embedded devices).

Answer 3

The loop is an excellent example of what not to do. ;)

---

Thread.currentThread().sleep(100);

There is no need to get the currentThread() as this is a static method. It is the same as

Thread.sleep(100);

---

catch (Exception e) {}

This is very bad practice. So bad I wouldn't suggest you put this even in examples, as someone might copy the code. A good portion of questions on this forum would be solved by printing out and reading the exception given.

---

You don't need to busy wait here. esp. when you expect to be waiting for such a long time.  Busy waiting can make sense if you expect to be waiting a very very short amount of time. e.g.

// From AtomicInteger
public final int getAndSet(int newValue) {
    for (;;) {
        int current = get();
        if (compareAndSet(current, newValue))
            return current;
    }
}

As you can see, it should be quite rare that this loop needs to go around more than once, and exponentially less likely to go around many times. (In a real application, rather than a micro-benchmark) This loop might be as short as 10 ns, which is not a long delay.

---

It could wait 99 ms needlessly. Say the producer is adding an entry 1 ms later, it has waited a long time for nothing.

The solution is simpler and clearer.

BlockingQueue<E> queue = 

E e = queue.take(); // blocks until an element is ready.

---

The list/queue is only going to change in another thread and a much simpler model for managing threads and queues is to use an ExecutorService

ExecutorService es =

final E e = 
es.submit(new Runnable() {
   public void run() {
       doSomethingWith(e);
   }
});

As you can see, you don't need to work with queues or threads directly. You just need to say what you want the thread pool to do.