4
\$\begingroup\$

Two parties communicate over a network sending back and forth up to 100 messages. Each message sent is a response to the last received message and depends on it. This means that both parties have to wait idly for the response to arrive before they can carry on with computations. Speed is quite critical and deteriorates very quickly when the network has moderate latency (due to the many messages and unavoidable waiting for the response). The messages are not very large, hence bandwidth does not seem to matter much.

Probably what I want to do can be achieved by using some library. If so, please point this out to me, preferably together with a demo or link to sources on how to use it. For lack of better alternatives I'm using (POSIX-API) TCP sockets here. I decided to use TCP rather than UDP because all data must be received in the correct order and the packet header size is not a relevant overhead, especially since latency is the issue.

This was my first time using sockets and I surely made many mistakes, both specific to sockets as well as pure C++ (I'm using C++17 although the code also compiles with C++11). My problem seems very standard and most of the code is puzzled together from some tutorials on sockets but I was struggling to find detailed sources on best practices.

Below is a simplified demo code that illustrates the way I handle the TCP logic. I tried to shorten it as much as possible but it is still quite long. Some comments:

  • tcp_helpers.h declares (AND defines for brevity of this post) functions containing all the TCP logic. The other two code files are an example application (main methods to run server and client). In my real code I encapsulate the TCP logic in classes, which internally call the functions shown here.
  • My messages can be variable size and have custom-defined headers specifying the length. The contents of a message is an array of custom-defined C-structs. Aside from these structs only having fixed-size primitive-type fields and no further structure, I would like my network code to work with any such user-defined struct-type. This leads to a big problem with portability: my code will probably not work if the two communicating systems use different byte order or different struct alignment. I am currently postponing this issue unless there is a straightforward way to take care of it.
  • I disable Nagle's algorithm to ensure the TCP packets are sent as soon as the message is ready. I learned about this from asking a Stackoverflow question.

Some questions I have already:

  1. The first version of my send_full_message function (see linked Stackoverflow question) was making two sys-calls to send, once for the (custom) header (8 byte struct) and once for the actual message (array of structs). In this version I reduced it to a single sys-call by copying header and data into a buffer (using perhaps ugly C-style memory manipulation). I did not notice a difference in performance compared to the original (sending the header as a separate packet). Which method is perferable? Can this be achieved more elegantly?
  2. The pointer arithmetic in the receive_structs function seems ugly. What would be the best-practice solution here?
  3. Is there anything else I could do to make this faster (just as I had not known about Nagle's algorithm before asking)?
// tcp_helpers.h.
// NOTE: Requires C++11, tested also with C++17. Using this code in the present form may be ill-advised.
// This is not a true header file (contains all method definitions for brevity).
#include <vector>
#include <iostream>
#include <string>
#include <sstream>
#include <cerrno>  // for checking socket error messages
#include <cstdint> // for fixed length integer types
#include <cstring> // for memcpy
#include <unistd.h>  // POSIX specific
#include <sys/socket.h> // POSIX specific
#include <netinet/in.h> // POSIX specific
#include <netinet/tcp.h> // POSIX specific
#include <arpa/inet.h> // POSIX specific

//////////////////// PROFILING ///////////////////
#include <chrono>

static auto start = std::chrono::high_resolution_clock::now();

// print a message with timestamp for rudimentary profiling. (I don't actually use this in my code)
void print_now(const std::string &message) {
    auto t2 = std::chrono::high_resolution_clock::now();
    std::chrono::duration<double> time_span = t2 - start;
    std::cout << time_span.count() << ": " << message << std::endl;
}
//////////////////// PROFILING ///////////////////

struct TCPMessageHeader { // Header for each message (I really use this).
    uint8_t protocol_name[4];
    uint32_t message_bytes;
};

struct ServerSends { // The server sends messages that are arrays of this struct (just an example).
    uint16_t a;
    uint32_t b;
    uint32_t c;
};

typedef uint8_t ClientSends; // The client sends messages that are arrays of this (just an example).

namespace TCP_Helpers {
    void disable_nagles_algorithm(int socket_fd) {
        const int enable_no_delay = 1;  // Disable Nagle's algorithm for TCP socket to improve performance
        if (setsockopt(socket_fd, IPPROTO_TCP, TCP_NODELAY, &enable_no_delay, sizeof(enable_no_delay))) {
            throw std::runtime_error("Failed to disble Nagle's algorithm by setting socket options");
        }
    }

    int init_client(const std::string &ip_address, int port) {
        int sock_fd;
        struct sockaddr_in serv_addr{};

        if ((sock_fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
            throw std::runtime_error("TCP Socket creation failed\n");
        }
        serv_addr.sin_family = AF_INET;
        serv_addr.sin_port = htons(port);
        // Convert IPv4 address from text to binary form
        if (inet_pton(AF_INET, ip_address.c_str(), &serv_addr.sin_addr) <= 0) {
            throw std::runtime_error("Invalid address/ Address not supported for TCP connection\n");
        }
        if (connect(sock_fd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) {
            throw std::runtime_error("Failed to connect to server.\n");
        }
        disable_nagles_algorithm(sock_fd);
        return sock_fd;
    }

    int init_server(int port) {
        int server_fd;
        int new_socket;
        struct sockaddr_in address{};
        int opt = 1;
        int addrlen = sizeof(address);
        // Creating socket file descriptor
        if ((server_fd = socket(AF_INET, SOCK_STREAM, 0)) == 0) {
            throw std::runtime_error("socket creation failed\n");
        }

        if (setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, &opt, sizeof(opt))) {
            throw std::runtime_error("failed to set socket options");
        }
        address.sin_family = AF_INET;
        address.sin_addr.s_addr = INADDR_ANY;
        address.sin_port = htons(port);
        // Forcefully attaching socket to the port
        if (bind(server_fd, (struct sockaddr *) &address, sizeof(address)) < 0) {
            throw std::runtime_error("bind failed");
        }
        if (listen(server_fd, 3) < 0) {
            throw std::runtime_error("listen failed");
        }
        if ((new_socket = accept(server_fd, (struct sockaddr *) &address, (socklen_t *) &addrlen)) < 0) {
            throw std::runtime_error("accept failed");
        }
        if (close(server_fd)) // don't need to listen for any more tcp connections (PvP connection).
            throw std::runtime_error("closing server socket failed");

        disable_nagles_algorithm(new_socket);
        return new_socket;
    }

    template<typename NakedStruct>
    void send_full_message(int fd, TCPMessageHeader header_to_send, const std::vector<NakedStruct> &structs_to_send) {
        const size_t num_message_bytes = sizeof(NakedStruct) * structs_to_send.size();
        if (header_to_send.message_bytes != num_message_bytes) {
            throw std::runtime_error("Trying to send struct vector whose size does not"
                                     " match the size claimed by message header");
        }
        print_now("Begin send_full_message");

        // copy header and structs vector contents to new vector (buffer) of bytes and sent via TCP.
        // Does not seem to be faster than sending two separate packets for header/message. Can the copy be avoided?
        std::vector<uint8_t> full_msg_packet(sizeof(header_to_send) + num_message_bytes);
        memcpy(full_msg_packet.data(), &header_to_send, sizeof(header_to_send));
        memcpy(full_msg_packet.data() + sizeof(header_to_send), structs_to_send.data(), num_message_bytes);

        // maybe need timeout and more error handling?
        size_t bytes_to_send = full_msg_packet.size() * sizeof(uint8_t);
        int send_retval;
        while (bytes_to_send != 0) {
            send_retval = send(fd, full_msg_packet.data(), sizeof(uint8_t) * full_msg_packet.size(), 0);
            if (send_retval == -1) {
                int errsv = errno;  // from errno.h
                std::stringstream s;
                s << "Sending data failed (locally). Errno:" << errsv
                  << " while sending header of size" << sizeof(header_to_send)
                  << " and data of size " << header_to_send.message_bytes << ".";
                throw std::runtime_error(s.str());
            }
            bytes_to_send -= send_retval;
        }
        print_now("end send_full_message.");
    }

    template<typename NakedStruct>
    std::vector<NakedStruct> receive_structs(int fd, uint32_t bytes_to_read) {
        print_now("Begin receive_structs");
        unsigned long num_structs_to_read;
        // ensure expected message is non-zero length and a multiple of the SingleBlockParityRequest struct
        if (bytes_to_read > 0 && bytes_to_read % sizeof(NakedStruct) == 0) {
            num_structs_to_read = bytes_to_read / sizeof(NakedStruct);
        } else {
            std::stringstream s;
            s << "Message length (bytes_to_read = " << bytes_to_read <<
              " ) specified in header does not divide into required stuct size (" << sizeof(NakedStruct) << ").";
            throw std::runtime_error(s.str());
        }
        // vector must have size > 0 for the following pointer arithmetic to work 
        // (this method must check this in above code).
        std::vector<NakedStruct> received_data(num_structs_to_read);
        int valread;
        while (bytes_to_read > 0)  // need to include some sort of timeout?!
        {
            valread = read(fd,
                           ((uint8_t *) (&received_data[0])) +
                           (num_structs_to_read * sizeof(NakedStruct) - bytes_to_read),
                           bytes_to_read);
            if (valread == -1) {
                throw std::runtime_error("Reading from socket file descriptor failed");
            } else {
                bytes_to_read -= valread;
            }
        }
        print_now("End receive_structs");
        return received_data;
    }

    void send_header(int fd, TCPMessageHeader header_to_send) {
        print_now("Start send_header");
        int bytes_to_send = sizeof(header_to_send);
        int send_retval;
        while (bytes_to_send != 0) {
            send_retval = send(fd, &header_to_send, sizeof(header_to_send), 0);
            if (send_retval == -1) {
                int errsv = errno;  // from errno.h
                std::stringstream s;
                s << "Sending data failed (locally). Errno:" << errsv << " while sending (lone) header.";
                throw std::runtime_error(s.str());
            }
            bytes_to_send -= send_retval;
        }
        print_now("End send_header");
    }

    TCPMessageHeader receive_header(int fd) {
        print_now("Start receive_header (calls receive_structs)");
        TCPMessageHeader retval = receive_structs<TCPMessageHeader>(fd, sizeof(TCPMessageHeader)).at(0);
        print_now("End receive_header (calls receive_structs)");
        return retval;
    }
}

// main_server.cpp
#include "tcp_helpers.h"

int main() {
    int port = 20000;
    int socket_fd = TCP_Helpers::init_server(port);
    while (true) { // server main loop
        TCPMessageHeader rcv_header = TCP_Helpers::receive_header(socket_fd);
        if (rcv_header.protocol_name[0] == 0)   // using first byte of header name as signal to end
            break;
        // receive message
        auto rcv_message = TCP_Helpers::receive_structs<ClientSends>(socket_fd, rcv_header.message_bytes);
//        for (ClientSends ex : rcv_message) // example "use" of the received data that takes a bit of time.
//            std::cout << static_cast<int>(ex) << " ";
//        std::cout << std::endl << std::endl;

        auto bunch_of_zeros = std::vector<ServerSends>(1000); // send a "response" containing 1000 structs of zeros
        TCPMessageHeader send_header{"abc", 1000 * sizeof(ServerSends)};
        TCP_Helpers::send_full_message(socket_fd, send_header, bunch_of_zeros);

    }
    exit(EXIT_SUCCESS);
}
// main_client.cpp
#include "tcp_helpers.h"

int main() {
    // establish connection to server and get socket file descriptor.
    int port = 20000;
    auto ip = "127.0.0.1";
    int socket1_fd = TCP_Helpers::init_client(ip, port);
    std::cout << "connected." << std::endl;
    for (int i = 0; i < 20; ++i) {  // repeat (for runtime statistics) sending and receiving arbitrary data
        // send a message containing 500 structs of zeros
        auto bunch_of_zeros = std::vector<ClientSends>(500);
        TCPMessageHeader send_header{"abc", 500 * sizeof(ClientSends)};
        TCP_Helpers::send_full_message(socket1_fd, send_header, bunch_of_zeros);

        // receive response
        TCPMessageHeader rcv_header = TCP_Helpers::receive_header(socket1_fd);
        auto rcv_message = TCP_Helpers::receive_structs<ServerSends>(socket1_fd, rcv_header.message_bytes);
//        for (ServerSends ex : rcv_message) // example "use" of the received data that takes a bit of time.
//            std::cout << ex.a << ex.b << ex.c << " ";
//        std::cout << std::endl << std::endl;
    }
    auto end_header = TCPMessageHeader{}; // initialized all fields to zero. "end" signal in this demonstration.
    TCP_Helpers::send_header(socket1_fd, end_header);
    exit(EXIT_SUCCESS);
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ You need to look up libevent. This is how you write responsive socket based applications. The problem is that most read/writes to a socket block (so you are wasting cycles). What you want to do is when you have a blocking call to read/write the thread has the opportunity to go and do other work (no hang idly waiting on the socket). Using libevent a single thread can easily handle thousands of simultaneous connections without blocking on any single conversation stream. \$\endgroup\$ – Martin York Aug 31 '20 at 17:53
  • 1
    \$\begingroup\$ Have a read of a the articles I wrote on sockets: lokiastari.com/series \$\endgroup\$ – Martin York Aug 31 '20 at 17:58
  • \$\begingroup\$ In my application there is no other useful work that can be done between computing a message to be sent and receiving a response. The response is required. Also there will only ever be two communicating parties. \$\endgroup\$ – Adomas Baliuka Aug 31 '20 at 18:08
  • \$\begingroup\$ Then simply build something on top of the simple curl interface. \$\endgroup\$ – Martin York Aug 31 '20 at 19:38
  • \$\begingroup\$ Now that I have read your code I would say there is an issue with keeping the same connection open. If it breaks your application is hard to reset to a know state. I would design it so that each message (request/reply) is independent and can thus by done over a new connection. This will make the application easier to reset to a known state if the connection is dropped/broken. Most client servers use this form of communication. This is then optimized at a lower level by allowing the reuse of an existing connection but that should be independent of the application layer. \$\endgroup\$ – Martin York Aug 31 '20 at 19:42
5
\$\begingroup\$

Overview

You use a single connection for all communication. This will make it hard to fix a broken/dropped connection. Better to make each message (request/response) its own connection. You can re-use connections under the hood but the application does not need to know this.

You are using a custom protocol. This is a bad idea. Use a standard protocol like HTTPS. This has a well defined and well supported set of libraries (that are all heavily tested). You can still use your binary messages on top of this.

You are using a binary protocol. Not an application killer but this will make the code much more brittle. I would use a human readable text protocol (especially when you are building a first version). The human readability will help in debugging. You can always switch to binary later if you can see a difference in speed.

I would use JSON over HTTP with TCP as the transport.

Now there is a cost to using all these layers. But I would argue that speed of development will be increased by using them. Once you have a working prototype then you can update your application and remove/replace each of the layers with an appropriate more effecient layer. But get it working first.

Look at Question

Two parties communicate

Summary Paragraph 1:

  • Lots of messages
  • Speed is critical
  • Dominated by network latency
  • Messages are large.

You sort of contradict yourself. Speed is critical but network latency is an issue. The only thing that matters is network latency. Any language can write to a socket much faster than the network can transport that answer. So writing reading is not a real speed critical thing (especially with small messages).

Now this can become an issue when you have a large messages and you make multiple large copies of the data then resources can be squeezed and this can have an effe t on speed.

Also you want to be efficient enough that the server can read messages from thousands of different sockets (many users) without causing any issues. so writing clear simple code that gracefully handle blocking calls would be a good idea.


Probably what I want to do can be achieved by using some library.

Yes you want to use a library. Which one depends on how low you want to go.

You can do it your self with libs like select()/pselect()/epoll(). Thats the basic hard rock ground. Its nice to understand this but probably not where you want to start.

The next level up is a library called libevent. This handles a lot of the low level details and is a thin wrapper over one of select()/pselect()/epoll(). It is still very low level but it abstracts away a couple of platform dependencies so makes writing multi platform code easier.

The next level up is probably libcurl. This has two interfaces. The Simple interface (great for clients). Make a request get data back from the request. The Multi interface great for servers. The multi interface makes writing servers that are handling multiple requests relatively simple.

I have written a lot of socket code that is avaialbe on the internet:

A couple of articles here:

There are examples to illustrate all these points in this github repo:

https://github.com/Loki-Astari/Examples

I have written a very basic wrapper around a socket that makes it wok just like a C++ std::istream:

https://github.com/Loki-Astari/ThorsStream/blob/master/doc/example1.md


I decided to use TCP rather than UDP because all data must be received in the correct order and the packet header size is not a relevant overhead, especially since latency is the issue.

Sure. Also UDP is broadcast so you are basically transmitting your data to the world. Also I am not sure you can use SSL with UDB so that becomes a real security issue.


Code Review:

Looks like you are using a binary protocl.

struct TCPMessageHeader { // Header for each message (I really use this).
    uint8_t protocol_name[4];
    uint32_t message_bytes;
};

Most systems nowadays have moved away from this. Binary protocols are very brittle and hard to change over time. A better bet is to use a soft human readable protocol like JSON. If you don't want to use a human readable one pick a binary protocol that is already supported (like BSON).


In C++ we put everything in a namespace for a reason. Use the C++ version of types not the C version.

struct ServerSends { // The server sends messages that are arrays of this struct (just an example).
    uint16_t a;         // std::unint16_t   << C++ version don't use the C
    uint32_t b;
    uint32_t c;
};

The Client object is an integer?

typedef uint8_t ClientSends; 

Also this is the old way of declaring a type-alias. Use the modern version it is simpler to read.

using ClientSends = std::uint8_t;

I have no idea what Nagle's algorithm is. But thanks for the name at least I can now look it up.

namespace TCP_Helpers {
    void disable_nagles_algorithm(int socket_fd) {
        const int enable_no_delay = 1;  // Disable Nagle's algorithm for TCP socket to improve performance
        if (setsockopt(socket_fd, IPPROTO_TCP, TCP_NODELAY, &enable_no_delay, sizeof(enable_no_delay))) {
            throw std::runtime_error("Failed to disble Nagle's algorithm by setting socket options");
        }
    }

If you are going to disable something. Then you need to explain why. Comments are great place to document "WHY" you are doing something. I would write an explanation about why "Nagle's" algorithm is causing speed issues and a docuemtned experiments on what you did to show this.

Most of what I read about Nagle's algorithm its a bad idea to turn it off.

But for real time communication is one of the few times where it would be useful. If this is your use case buffering the message like you do then sending it as a single object (rather than getting each object to write to the stream) and disabling Nagle's algorithm seem to be the best choice. But saying that its not clear from your code that this is necessary so please add some detailed documents about why you are disabling Nagle's algorithm.


The init_client() looks good.

This is supposed to zero initialize the structure.

        struct sockaddr_in serv_addr{};     // That guarantees a zero - init
                                            // I would have to look up if that
                                            // is the same as a zero fill with
                                            // C structures. 


        struct sockaddr_in serv_addr = {0}; // That guarantees a zero fill.

Try not to use C like cast.

        if (connect(sock_fd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) {

Make it vey obvious in your code that you have a dangerious cast by using the C++ version that stickout like a saw thumb and makes sure your code is givent the required scruteny.

        if (connect(sock_fd, reinterpret_cast<sockaddr*>(&serv_addr), sizeof(serv_addr)) < 0) {

The intit_server() is doing more than it should. You are also leaking the original socket file descriptor.

The call to accept() creates a new socket connection. But the original socket server_fd is still open and listening (though you don't have anybody listening). The normal pattern would be more like this:

    initServer()
         server_fd = socket();
         bind(server_fd);

         while(!finished)
         {
             listen(server_fd);
             new_socket = accept(server_fd);

             workerQueue.add(newSocket);  // You then have another **SINGLE** thread
                                          // that handles all the worker queue
                                          // sockets



                                          // If you use a library like libevent
                                          // You can do this and all the connections
                                          // with the same thread.
         }
         close(server_fd);

Not sure that copying the data into a single message byes you anything.

        std::vector<uint8_t> full_msg_packet(sizeof(header_to_send) + num_message_bytes);
        memcpy(full_msg_packet.data(), &header_to_send, sizeof(header_to_send));
        memcpy(full_msg_packet.data() + sizeof(header_to_send), structs_to_send.data(), num_message_bytes);

The sockets are themselves already buffered. So you are copying the data into a buffer then writing the buffer to the socket which is buffering up the writes. The advantage to you is that it makes writing the below loop easier. The disadvantage is that your objects need to be plain old data. It would be nice to have objects that know how to serialize themselves to the socket stream.

** Have read a bit more about comms. This is a good idea if you have disabled Nagle's algorithm as it will create the optimally sized packages and thus reduce the overhead of the TCP/IP package header. You are basically taking over the job of the algorithm and doing the buffering.


Stop using C algorithms when there are much better documented C++ versions:

        memcpy(full_msg_packet.data(), &header_to_send, sizeof(header_to_send));

        // could be written as:

        // may need to add some casts or access functions.
        std::copy(&header_to_send, &header_to_send + sizeof(header_to_send), full_msg_packet.data());

     

BUG HERE

You don't use how many bytes you have already sent. So if it requires multiple calls to send() then you are resening some of the data.

            send_retval = send(fd, full_msg_packet.data(), sizeof(uint8_t) * full_msg_packet.size(), 0);


            // Should be:
            bytesAlreadySent = 0;

            ...

            send_retval = send(fd,
                               full_msg_packet.data() + bytesAlreadySent,
                               sizeof(uint8_t) * full_msg_packet.size() - bytesAlreadySent,
                               0);

            ....

            bytesAlreadySent += send_retval;

Common issue here:

            if (send_retval == -1) {
                int errsv = errno;  // from errno.h
                std::stringstream s;
                s << "Sending data failed (locally). Errno:" << errsv
                  << " while sending header of size" << sizeof(header_to_send)
                  << " and data of size " << header_to_send.message_bytes << ".";
                throw std::runtime_error(s.str());
            }

Not all errors are catastrophic. Some errors are programming bugs and should be found during testing and removed. If these happen in production you need to stop the application by throwing a non catchable exception. Others are real problems that you should simply throw an exception for but there is a third set which simply mean the system was sudeenly busy. In these you should simply retry the send.


Why are you making a copy of the header object?

    void send_header(int fd, TCPMessageHeader header_to_send) {

Pass by const reference.


\$\endgroup\$
4
  • \$\begingroup\$ Thank you very much for the detailed answer. I will look at the libraries you suggested. In your code review I did not understand what you mean by "I leak the server file descriptor". If close the associated socket (which I do), the server is no longer listening for connections, right? In my use case there will only ever be two communicating parties. The server need not listen for further connections (at least until the existing connection is broken), which I take to mean that using a queue or threads has no benefit. \$\endgroup\$ – Adomas Baliuka Sep 2 '20 at 8:58
  • \$\begingroup\$ The question of how to correctly zero-init/zero-fill C-structs in C++ seems more complicated than I thought (see, e.g., stackoverflow.com/a/61240590/9988487). Still not sure what is best to use as default, although in the present case T var{};, T var = {}; and T var = {0}; (suggested by the answer) seem to give the same results. \$\endgroup\$ – Adomas Baliuka Sep 2 '20 at 13:27
  • \$\begingroup\$ @AdomasBaliuka There are some subtitles. Zero-init: Will initialize each member to a zero value (assuming they are all POD (this case holds; it is a C structure)). Zero-fill: Will initialize the memory used by the object to all zero bytes. All things being equal these should be the same thing. BUT there are parts of the object that may not be covered by members (the padding). If there is any padding in the object then this may potentially be left uninitialized. Since this is a C object I would stick to the C way of initializing it (like all the examples on the web) and use T var = {0}; \$\endgroup\$ – Martin York Sep 2 '20 at 17:51
  • \$\begingroup\$ @AdomasBaliuka You are correct you don't leak the server file descriptor. I missed that in your code. I have struck out that comment. But left in the part about normal use case for future engineers that would read the code. \$\endgroup\$ – Martin York Sep 2 '20 at 17:55
3
\$\begingroup\$

I agree with most of what Martin York wrote, except perhaps the remark about binary protocols. Sometimes sending structs is just the right thing to do: it's very fast, reasonably compact, and requires no conversion to and from some other format, which can waste CPU cycles and perhaps require lots of external dependencies. But, unless you think about extensibility up front, you can easily lock yourself into a set of structs without the possibility to migrate gracefully to newer versions. Your code only handles structs for which the size is known up front. You might consider adding functionality to handle "structs" with a variable size.

Apart form that I just want to add these things:

  1. The first version of my send_full_message function (see linked Stackoverflow question) was making two sys-calls to send, once for the (custom) header (8 byte struct) and once for the actual message (array of structs). In this version I reduced it to a single sys-call by copying header and data into a buffer (using perhaps ugly C-style memory manipulation). I did not notice a difference in performance compared to the original (sending the header as a separate packet). Which method is perferable? Can this be achieved more elegantly?

There is a third option which uses only one syscall and does not require copying the data, and that is by using sendmsg. It allows you to specify a list of discontiguous memory regions that need to be sent over a socket as if it was one contiguous block. It requires some more lines of code to set up the structs necessary to pass to sendmsg(), but some of them can perhaps be prepared once and then reused.

  1. Is there anything else I could do to make this faster (just as I had not known about Nagle's algorithm before asking)?

Disabling Nagle's is trading in bandwidth for latency. Instead of doing this, consider using TCP_CORK. When the application knows it wants to send a bunch of data, and wants the packets to be sent off without delay but with the best use of the network MTU as possible, it should enable TCP_CORK at the start of that bunch of data, and when it sent everything, it disables TCP_CORK, which will then ensure that any remaining data in the send buffer will be sent immediately (assuming the congestion window allows it). If you would disable Nagle instead, and want to send lots of small structs in a row, then each struct would be sent as a separate packet for no good reason.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.