Decrease CPU usage in while loop

Question

I've a application which runs with two threads. The main thread runs in one infinity-while-loop:

while (true) {
 try {
 ArrayList<Info> infoList = zabbixHandler.APIRequest();
 handleInfo.handleInformation(infoList);
 Thread.sleep(5000);
 }
}

The second thread, which uses more than 50% of my CPU, must be able to stop and run again, which I asked already with this question on Stack Overflow.

The solution works perfectly, but as I already said, it uses>50% of the CPU. Here are the two loops of the solution:

private volatile boolean finished = true;
public void run() {
 Process p;
 while (true) {
 // loop until the thread is finished
 while (!finished) {
 try {
 // let every lamp shine...
 p = Runtime.getRuntime().exec(onCommand + "-green");
 p.waitFor();
 // ... for 0.5 seconds ...
 Thread.sleep(500);
 // ... then turn it off again ...
 p = Runtime.getRuntime().exec(offCommand + "-green");
 p.waitFor();
 // ... and let the next lamp shine
 p = Runtime.getRuntime().exec(onCommand + "-yellow");
 p.waitFor();
 Thread.sleep(500);
 p = Runtime.getRuntime().exec(offCommand + "-yellow");
 p.waitFor();
 p = Runtime.getRuntime().exec(onCommand + "-red");
 p.waitFor();
 Thread.sleep(500);
 p = Runtime.getRuntime().exec(offCommand + "-red");
 p.waitFor();
 p = Runtime.getRuntime().exec(onCommand + "-blue");
 p.waitFor();
 Thread.sleep(500);
 p = Runtime.getRuntime().exec(offCommand + "-blue");
 p.waitFor();
 } catch (InterruptedException e) {
 e.printStackTrace();
 } catch (IOException e) {
 e.printStackTrace();
 }
 }
 }
}

How could I reduce the use of CPU while retaining the functionality?

EDIT:
The finish element will be changed by a getter and setter. Because it is volaitle, it shouldn't get complications.

• Malachi: please provide the actual code inside the second code block.
• Bobby: Are you aware that Thread.sleep(milliseconds) is a rough estimation only?
• Malachi: where does finished change to true?
• rolfl: How are you measuring that the second thread takes > 50% of the time?
• rolfl: -1 vote because you are not showing us the most important part of the code: how does the variable finished get declared, and how is it modified. Will remove -1 when question is corrected.

Answer 1

don't use a busy wait but instead use a wait condition (here using the built-in monitor in Object):

public void run() {
 while(true){
 while(!finished){
 // do stuff
 }
 try{
 synchronized(this){
 while(finished)
 wait();//wait until notify gets called in startThread
 }
 }catch(InterruptedException e){}
 }
}
public synchronized void startThread() {
 finished = false;
 notify();//wake up the wait
}

I note that finished should remain volatile.

I put the wait() in the synchronized block in a while loop to avoid the race where the thread has just tested the first while condition and is about to enter the synchronized block but startThread() then gets called; putting finished back to false and the thread waiting anyway. This can lead to the thread blocking while waiting on a notify() that never happens.

To help any more I need to see what exactly happens inside that while loop.

• rolfl: This answer makes no sense to me.... where is there a spin lock?
• ratchet freak: @rolfl when finished is true the thread keeps spinning in the outer while but never getting in the inner while, this is equivalent to while(true){while(finished){}/*do stuff*/}
• rolfl: Ahh, that makes sense, but why didn't you say that in your answer? ... ;-)
• rolfl: Assuming you are right about the finished variable (which I suspect you are), then the better/right/modern solution would be using Reentrant-lock and conditions.
• ratchet freak: @rolfl maybe but I think that's a bit overkill in this case

Answer 2

The right tool for this job in Java6 and newer is the combination of a ReentrantLock and a Condition

EDIT: You should also consider changing your Process-running code.... consider creating a function:

private static void runAndWaitFor(String command) {
 Process p = Runtime.getRuntime().exec(command);
 p.waitFor();
}

Then, in your blinking loop, you can reduce your code duplication with:

 // let every lamp shine...
 runAndWaitFor(onCommand + "-green");
 // ... for 0.5 seconds ...
 Thread.sleep(500);
 // ... then turn it off again ...
 runAndWaitFor(offCommand + "-green");
 // ... and let the next lamp shine
 runAndWaitFor(onCommand + "-yellow");
 ...... etc. .....

Now, even with the simpler code, you still have the two threads, one needs to wait until a condition is met, and when the condition is met, it needs to execute some work until the condition is reverted.

In your 'working' thread you want a method to call workStarted() which will start the lights blinking, and then workCompleted() which will stop the lights.

The pattern to use is:

import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;
public class ThreadBlocker implements Runnable {
 private final ReentrantLock mylock = new ReentrantLock();
 private final Condition workready = mylock.newCondition();
 private boolean finished = true;
 public void workStarted() {
 mylock.lock();
 try {
 finished = false;
 workready.signalAll();
 } finally {
 mylock.unlock();
 }
 }
 public void workCompleted() {
 mylock.lock();
 try {
 finished = true;
 } finally {
 mylock.unlock();
 }
 }
 private void waitForWork() throws InterruptedException {
 mylock.lock();
 try {
 while (finished) {
 workready.await();
 }
 } finally {
 mylock.unlock();
 }
 }
 public void run () {
 try {
 while (true) {
 // the following method will block unless there is work to do.
 waitForWork();
 // change your lights in here.
 }
 } catch (InterruptedException ie) {
 // do some handling for the thread.
 ie.printStackTrace();
 Thread.currentThread().interrupt();
 }
 }
}

• ratchet freak: you don't need to signalAll in workCompleted()
• Malachi: is this sort of like having two objects where one goes and when it's done it starts (calls) the other, and when the other is done it starts (calls) the first? creating it's own loop
• rolfl: Not really, this is like a motion-sensitive security light. If there is motion, it turns the light on for a few seconds. At the end of that time, if there is ongoing motion, it stays on for a few more seconds. If there is no motion, it turns off.