Huge performance improvement

[pliniofpa]

Hi,
I am working on a replacement for a 30+ years old IPC tool and I am investigating if D-Bus meets the performance that we see with our current (old) solution.

I understand that D-Bus is not meant to carry a lot of data from everything I read online but at this moment my job is only to compare how fast our old system can move data vs how fast can D-Bus move the same amount of data.

I started my testing using a std::vector<uint8_t> with 20*1024 items. I then call a method that receives that vector in a loop for 10000 times. So I am moving a total of 200MB of data and I measure how long it takes (in wall clock and CPU clock).

For my surprise this code was taking over 15 seconds (wall clock) to execute while our current IPC solution takes only 6 seconds. Even more intriguing, CPU was being consumed for ~10 seconds (out of the 15 total wall clock seconds).

Investigating the source code I realized that, under the hood, sdbus-c++ is sending item by item of a vector using sd_bus_message_append_basic() instead of sending the whole data at once using sd_bus_message_append_array().

I modified the source code of sdbus-c++ to use sd_bus_message_append_array() and sd_bus_message_read_array() when dealing with std::vector<> of types int16_t, int32_t, int64_t, uint8_t, uint16_t, uint32_t, uint64_t, and double as the specification allow for those types to be sent as arrays.

When retesting with my local mods the same code mentioned above took less than 3 seconds to execute with less than 0.5 second spent by CPU. This is a great improvement.

Please consider the Pull Request I am submitting with those changes.

Here is the whole code used for testing:

#include <sdbus-c++/sdbus-c++.h>
#include <vector>
#include <string>
#include <Timer.h> 

#define TYPE uint8_t
constexpr auto totalItems = 20*1024;
constexpr auto numberOfCalls = 10000;

void byteArrayMethod(sdbus::MethodCall call) 
{    
    //TYPE result = 1; 
    // Reply
    auto reply = call.createReply(); 
    //reply << result;
    reply.send();
}

int main(int , char **) {    
    std::cout << "Starting application" << std::endl;
    
    // Create D-Bus connection to the system bus and requests name on it.
    const char* serviceName = "com.oracle.nms.byteArray";
    auto connection = sdbus::createSessionBusConnection(serviceName);
    
    const char* objectPath = "/com/oracle/nms/byteArray";
    const char* interfaceName = "com.oracle.nms.ByteArray";
    const char* destinationName = "com.oracle.nms.byteArray";   
    
    std::thread clientThread([&](){
        // Sleep so the server can start 
        std::this_thread::sleep_for(std::chrono::milliseconds(100));        
        std::cout << "Starting client" << std::endl;       
        // Create proxy object for the concatenator object on the server side        
        auto byteArrayProxy = sdbus::createProxy(*connection, destinationName, objectPath);        
        byteArrayProxy->finishRegistration();        
        std::vector<TYPE> arrayOfItems(totalItems);
        
        CPUTimer timer;
        timer.start(); 
        for(size_t i=0;i<numberOfCalls;++i)        
        {             
            auto method = byteArrayProxy->createMethodCall(interfaceName, "byteArray");            
            method << arrayOfItems;            
            auto reply = byteArrayProxy->callMethod(method);
//            TYPE result = 0;
//            reply >> result;
//            std::cout << "Replied = " << result << std::endl;
        }    
        timer.stop();        
        auto wall = timer.walltime();
        auto cpu = timer.cputime();
        auto wall_micro = (double)wall.tv_sec*1000000 + (double)wall.tv_nsec/1000;        
        std::cout << "Done calling method. Time wall = " << wall << " secs (" << 
                     wall_micro/numberOfCalls << " uSec per call). Time CPU = " << 
                     cpu << " secs" << std::endl;
        // Stay here
        sleep(10);              
    });    
    std::thread serverThread([&](){        
        std::cout << "Starting server" << std::endl;        
        // Create byteArray D-Bus object.
        auto concatenator = sdbus::createObject(*connection, objectPath);
        // Register D-Bus methods and signals on the concatenator object, and exports the object.
        concatenator->registerMethod(interfaceName, "byteArray", "ay", "", &byteArrayMethod);
        concatenator->finishRegistration();        
        // Stay here
        sleep(100);
    });    
    // Event loop no blocking
    connection->enterEventLoop();    
    clientThread.join();    
    serverThread.join();    
    return 0; 
}

[dleeds-cpi]

@pliniofpa this is a great observation, and I am sure this optimization for data contained in a std::vector will be a good addition to the library.

That said, D-Bus is a remote procedure call framework and I would advise against using D-Bus messages for high bandwidth data transfer. All messages must be proxied through a broker service and this introduces unnecessary overhead when sending a large amount of data from process A to process B. Instead, D-Bus provides the ability to open a dedicated channel for high-bandwidth data exchange. It does this by supporting a Unix file descriptor data type (h). To use this, you might add a UNIX_FD OpenDataPipe() D-Bus method call to your service, and return one end of a Unix socket pair to the caller. The client can then send and receive data directly with the service in a very efficient manner.

Also a note on the above test code. The serverThread is not doing anything useful, so you might as well get rid of it.

[pliniofpa]

@dleeds-cpi,
Thanks for your comments.
I intentionally mentioned that DBus is not supposed to be caring too much data by itself in my original post because I knew I would hear that I was misusing it. However, that doesn't change the fact that there is a more efficient/correct way of sending an array of data.
The test application is only a test application for this post and the serverThread is creating/acquiring the Bus used by the client thread. This application will not be used.

Thanks again.

[sangelovic]

Thanks @pliniofpa for coming up with this and for the PR. I'm now looking into it. Thanks @dleeds-cpi for additional comments.

[sangelovic]

#340 has been merged.