doip / doip

Diagnostics over IP
GNU Lesser General Public License v3.0
27 stars 19 forks source link

use Socket.setTcpNoDelay(true); #36

Closed stevenfoot closed 4 years ago

stevenfoot commented 4 years ago

UDS uses a lot of small messages and waits for the response befor the next response is send. By default a Tcp stack waits some time before a message is send to bundle several messages. This leads to bad perfomance. If Socket.setTcpNoDelay(true) is used the simulation works 2-3 times faster for some usecases. This TCP setting is also used in real ecus.

marco-wehnert commented 4 years ago

This option is already set for TCP connections in the class doip.simulation.standard.StandardGateway. This function will be called when a new TCP connection had been accepted in the server socket (see code at the end). Is there any other place where we forgot to call setTcpNoDelay(true)? What had not be done is to call this function also in the unit tests where a socket will be created (That would be the tester side). Can you give me a hint where you changed the setting to get the simulation running faster?

We have two options: A) We keep it as it is, there is the one place in the StandardGateway where the function will be called. It makes it easy to override if someone does not want to call setTcpNoDelay(). We could implement one function StandardGateway.getTcpNoDelay() which returns per default true, if user want to set it to false he can override the function getTcpNoDelay() -> That means finally the doip-library doesn't manage this flag. A flag in the property file makes also sense. B) We setTcpNoDelay in one of the start() functions where the socket is given as parameter in the doip-library. Developer of a custom simulation does not need to take care. But it might be more difficult to change the setting because it is in a very low level, if you want to override it you first must override all classes between the gateway and one of the low level classes in the doip-library. A setting in a property file is not easy, as per convention the doip-library does not know anything about a property file.

I personally prefer option A). We could add a paragraph in the documentation (Javadoc or in the PDF).

public void onConnectionAccepted(TcpServer tcpServer, Socket socket) {
    logger.trace(">>> void onConnectionAccepted(Socket socket)");

    try {
        socket.setTcpNoDelay(true);
    } catch (SocketException e) {
        logger.error(Helper.getExceptionAsString(e));
    }

    StandardTcpConnectionGateway standardConnection = createConnection();
    standardConnection.addListener(this);
    this.standardConnectionList.add(standardConnection);
    standardConnection.start(socket);
    logger.trace("<<< void onConnectionAccepted(Socket socket)");
}
stevenfoot commented 4 years ago

Checked it again. You are right. Allready implemented. No need for change.