Java server structure

Question

Currently my java server is working fine, stores all clients on a new thread, but is not that good at perfomance.

My MainServer class

public class MainServer {

    private ServerSocket server;

    public MainServer(int port) {

        try {

            server = new ServerSocket(port);
            System.out.println("Server started on port " + port + "...");
            ServerLoop();

        } catch (IOException e) {

            System.out.println("[ERROR] Server cannot be started on port " + port + "...");
        }
    }

    public void ServerLoop() {

        Thread serverLoop = new Thread(new Runnable() {

            public void run() {

                ServerDispatcher dispatcher = new ServerDispatcher();

                while (true) {

                    try {

                        Socket connectedClient = server.accept();
                        dispatcher.connectedClients.add(connectedClient);
                        System.out.println("Client number - " + dispatcher.connectedClients.size() + " connected...");
                        Thread clientThread = new Thread(new ClientHandler(connectedClient, dispatcher));
                        clientThread.start();

                    } catch (IOException e) {

                        e.printStackTrace();
                    }
                }
            }
        });

        serverLoop.start();
    }
}

So this part will start a new thread for each client and add the client to the dispatcher client list

ClientHandler class

public class ClientHandler implements Runnable {

    private Socket currentClient;
    private ServerDispatcher handler;

    public ClientHandler(Socket s, ServerDispatcher dispatcher) {

        currentClient = s;
        handler = dispatcher;
    }

    public void run() {



        while (true) {


        }


    }
}

And this is my ServerDispatcher class

public class ServerDispatcher {

    public ArrayList<Socket> connectedClients;

    public ServerDispatcher() {

        connectedClients = new ArrayList<Socket>();
        CheckClients();
    }

    public void CheckClients() {

        Thread checkClients = new Thread(new Runnable() {

            public void run() {

                while (true) {

                    Integer clientSize = connectedClients.size();

                    if (!clientSize.equals(0)) {

                        for (int i = 0; i < connectedClients.size(); i++) {

                            try {

                                BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(connectedClients.get(i).getOutputStream()));
                                writer.write("1");
                                writer.flush();
                                //System.out.println("Checking");

                            } catch (IOException e) {

                                int clientNumber = i + 1;
                                System.out.println("Client number - " + clientNumber + " disconnected...");
                                connectedClients.remove(i);
                            }
                        }
                    }
                }
            }
        });

        checkClients.start();
    }
}

And this checks for all online clients...

The problem is this server is using like 67% CPU at execution, how can I improve the code? maybe removing all the threads from ServerDispatcher and just leave the client thread?

And another question: I would like to send a message to X client on my while loop but just for ONE time, I looked on google and it seems that LinkedBlockingQueue is what Im looking for but I cant find any tutorial for a easy implementation..

Answer 1

Your loop just sends data as fast as it can and busy waits when it can't. This is bound to use lots of CPU. A simple solution is to only send dummy data periodically (since it doesn't really need to be sent at all)

I suggest adding

Thread.sleep(100);

to the sending loop and your CPU should drop to around 1%.

There is a number other improvements you could make but none anywhere near as significant.