0

I need to continuously listen to a remote socket and react on given input.

public void listen(String ip, int port) {
    try (
      Socket socketListener = new Socket(ip, port);
      BufferedReader portReader = new BufferedReader(new InputStreamReader(socketListener.getInputStream()));
    ) {
      while (true) {
        while (!portReader.ready()) {
          // Wait for next PORT message
        }

        Logger.log(LogComponent.SOCKET, "Event received");
      }
    }
  }

What am I doing so enormously wrong that the code above is using 100% CPU load?

While debugging I can see that the while-!portreader-loop is the evildoer. But most of the examples I've found are doing it the same way.

EDIT #1

Considering your comments I have following solution right now:

try {

  Socket SocketListener = new Socket(ip, port);
  BufferedReader portReader =
    new BufferedReader(
      new InputStreamReader(SocketListener.getInputStream())
    );

  // We do not use common while(true)-pattern for reading the input.
  // Instead, we check for new input 3 times a second.
  ScheduledExecutorService executor = Executors.newScheduledThreadPool(10);
  executor.scheduleAtFixedRate(() -> {
    try {
      processInput(portReader);
    } catch (IOException e) {
      e.printStackTrace();
    }
  }, 0, 333, TimeUnit.MILLISECONDS);

} catch (Exception exception) {
  exception.printStackTrace();
}

And processInput(0) is doing the action now. This ends in better performance results than using simply Thread.sleep() - though I don't understand why.

When using this approach: Can the code miss some messages from the socket? I mean during the intervals?

Christopher Will
  • 2,991
  • 3
  • 29
  • 46
  • 1
    `while true` will always run as fast as your CPU allows. Thus a `delay` is needed to slow down your loop. Calculate how often per second you want to check the remote socket and insert an appropriate delay. E.g waiting 1/10 of a second would result in about 10 checks per second (minus check time). – moestly Oct 18 '17 at 10:46
  • You also need to provide a byte that marks the end of a message. The way your code is designed read will never return -1, you would have to close the stream for this to happen. – MissingSemiColon Oct 18 '17 at 11:00
  • Why can't you just perform a blocking read? Using `Reader.ready()` to test for readability is a wrong design. If you need non-blocking behavior consider using SelectableChannels, or perhaps the asynchronous I/O features introduced in Java 7. – President James K. Polk Oct 18 '17 at 20:11

2 Answers2

1

Your CPU is busy because it's processing instructions in the while loop.

To avoid it, you should use a function that waits for the socket to be connected. If you are waiting for incoming connection, use Socket.accept(). This will block the thread (i.e. thread won't be scheduled for execution) until connection is established.

Do not use Thread.sleep() as others have suggested. While this does lower the CPU usage somewhat, it will still burn CPU unnecessarily, as well as introduce a latency. This is a bad engineering practice.

Apart from that, you might want to look into non-blocking or asynchronous I/O. See here for more information.

jurez
  • 4,436
  • 2
  • 12
  • 20
  • The server almost immediately accepts the socket, this is not the issue. The socket then stays open continuously. But it is sending data just every couple of seconds. Thanks for the link! – Christopher Will Oct 18 '17 at 12:37
  • In that case you can try `portReader.read()` which should block (release thread) until input is available. As for your EDIT #1, you have now wasting CPU on another thread, it's still not the solution you are looking for, it's actually even worse than before. – jurez Oct 18 '17 at 13:00
  • See also https://docs.oracle.com/javase/tutorial/networking/sockets/clientServer.html – jurez Oct 18 '17 at 13:01
0

The problem is that your code inside your while(true) is consuming all the CPU. Make a single change like:

public void listen(String ip, int port) {
    try (Socket socketListener = new Socket(ip, port);
    BufferedReader portReader = new BufferedReader(new InputStreamReader(socketListener.getInputStream()));) {
        while (true) {
            while (!portReader.ready()) {
              // Wait for next PORT message
                try {
                    Thread.sleep(1);
                } catch(InterruptedException e) {
                    //handle InterruptedException
                }
            }
            Logger.log(LogComponent.SOCKET, "Event received");
        }
    }
} 
Duloren
  • 2,395
  • 1
  • 25
  • 36