Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation takes a long time #910

Open
nandlab opened this issue Dec 7, 2021 · 19 comments
Open

Compilation takes a long time #910

nandlab opened this issue Dec 7, 2021 · 19 comments

Comments

@nandlab
Copy link

nandlab commented Dec 7, 2021

Hello,

building a program with MQTT_CPP takes very long. I am adding mqtt_cpp as a git submodule.
The following example program takes 35 (!) seconds to compile on my linux machine:

#include <iostream>
#include "mqtt_client_cpp.hpp"

using namespace std;

int main()
{
    cout << "Hello World!" << endl;
    return 0;
}

This is my CMake file:

cmake_minimum_required(VERSION 3.5)

project(MqttCppTestInclusion LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

add_executable(MqttCppTestInclusion main.cpp)
set(MQTT_CPP_INCLUDE "${CMAKE_CURRENT_SOURCE_DIR}/mqtt_cpp/include")
include_directories(${CMAKE_SOURCE_DIR} ${MQTT_CPP_INCLUDE})
set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)
find_package(Boost REQUIRED system)
target_link_libraries(MqttCppTestInclusion PRIVATE Threads::Threads Boost::system)
target_link_libraries(MqttCppTestInclusion LINK_PUBLIC)

Even if I make a small change somewhere which is not related to MQTT, it always takes the same time to recompile.

Is there a way to compile programs with mqtt_cpp more efficiently?

Could maybe separating the function declarations and definitions in mqtt_cpp into separate files increase the incremental build speed?

@redboltz
Copy link
Owner

I understand the issue.
I just investigated the file that takes longer compile times.
I think that property_variant.hpp is one of them.

#include <iostream>
#include "mqtt/property_variant.hpp"

using namespace std;

int main()
{
    cout << "Hello World!" << endl;
    return 0;
}

How long does it takes on your environment?

@AndreaRicchi
Copy link
Contributor

Hi,
I have the same issue on a bigger project. Every class that include the MQTT client takes a very long time to build. I've traced the build steps for a file and the result is that 90% of the time is used for the mqtt/client.hpp class.

Here the CLang trace JSON. You can open it in chrome://tracing: pushactionrunner.cpp.json.gz

I tried bumping to the v12 version, using PCH, using STD types instead of BOOST; but nothing improved the build times.

@redboltz
Copy link
Owner

@AndreaRicchi

I opend the trace using chrome. I'm not sure how to use it.

I have the same issue on a bigger project. Every class that include the MQTT client takes a very long time to build. I've traced the build steps for a file and the result is that 90% of the time is used for the mqtt/client.hpp class.

#include <iostream>
// Switch one of them
#include "mqtt/client.hpp"
#include "mqtt/endpoint.hpp"
#include "mqtt/property_variant.hpp"

using namespace std;

int main()
{
    cout << "Hello World!" << endl;
    return 0;
}

Do you mean only mqtt/client.hpp case takes a long time and mqtt/endpoint.hpp and mqtt/property_variant.hpp take a short time?

@AndreaRicchi
Copy link
Contributor

@redboltz

Based on the flame graph there are 2 issues. The first one is the source parsing. In particular the mqtt/client.hpp took 6 seconds. Off these 6 seconds, 4 are used for the mqtt/endpoint.hpp.

The second issue is related to the template function istantiation. You can follow this link on how to read that file.

@redboltz
Copy link
Owner

Thank you for the information!
I will take a look it.

Currently, my priority is runtime performance.
Compile time is actually long but I personally can accept it. But if there is a good way to increase the speed without runtime performance penalty and flexibility penalty, I will apply it. I beleive that there is some good way but I'm not an expert of compiling time performance tuning. So I think that I might need long time to analyse it and find a solution. Pull requests are welcome :)

@nandlab
Copy link
Author

nandlab commented Dec 16, 2021

@redboltz

I understand the issue. I just investigated the file that takes longer compile times. I think that property_variant.hpp is one of them.

If I write the following include statement in the hello world program, the compilation takes 7 seconds in my environment:

#include "mqtt/property_variant.hpp"

@AndreaRicchi
Copy link
Contributor

@redboltz

I know that compile time is not as important as runtime performance. But when the library is used in a more complex application where, as in my example, the MqttClient is store as a class member. This means that all the classes that need to send a MQTT messages will include that header and the build time increase tremendously.
At the moment I solved this issue moving all the mqtt_cpp references in the source file and using the MqttClient as a static variable instead of class member, but is not C++ style. Maybe we can add a more complex example using classes specifying that using any library include in the class header could heavily impact on the compile time.
If you agree on that a I can create a PR with the example. 😄

@nandlab
Copy link
Author

nandlab commented Dec 23, 2021

@redboltz

Currently, my priority is runtime performance.
Compile time is actually long but I personally can accept it. But if there is a good way to increase the speed without runtime performance penalty and flexibility penalty, I will apply it.

How is compile time related to runtime performance?

Normally, in the most libraries, headers, which are included in the user program, are lightweight and only contain the necessary function declarations.
The actual function definitions, on the other hand, are in separate library source files (*.c and *.cpp). They have to be build once, so that a static library archive (*.a) or a shared library (*.so) is created.
Then the user program does not have to compile the library code every time, instead it just links to it (statically or dynamically).

If you work on a larger project, where every file includes (mostly indirectly) a header, which takes half a minute to compile, the total compile time becomes huge. Here's a project I work on: RoomControl. On my machine it takes 2 minutes to compile, where most of the time is caused by the mqtt_client_cpp.hpp header.

I also tried out the paho.mqtt.cpp library. This demo program with paho.mqtt.cpp takes 2 seconds to compile.

The mqtt_cpp library can be very useful, because it integrates with Boost ASIO, which can also be used for other asynchronous functionality. But unfortunately, the compile time right now makes the development in my application very hard.

Unfortunately I can not provide a pull request, because I do not know the library well enough to optimize it.

It would be very nice, if you could find the time to split this useful library into separate *.hpp headers (only with declarations) and *.cpp source files.

@redboltz
Copy link
Owner

How is compile time related to runtime performance?

For example, function calls via virtual function table could affect runtime performance.
So I use template based approach. It tends to increase compile time but good runtime performance.
However, I understand a combination of MQTT_ALWAYS_INLINE and virtual function in the same compilation unit (implementation is in the same cpp file) can be inlined.
It is just an example. There are lot of similar tricks.

I have no plan to provide a library files on mqtt_cpp. Header only is important for me, so far.
If separated library files are provided, many trouble would happen.
Different compile option make SEGV easily on the bad combination of so and application program.
It's easy to predict users report the library doesn't work on their environment. I want to avoid it.

In addtion, I think that the Boost Libraries tend to be header only as long as it can.

I'm looking for decreasing compile time way on header only library. The most of boost libraries achieve that, so I guess there is a way. I will analyze using the tool introduced by @AndreaRicchi and will try to find a solution.
I need time to do that.

@AndreaRicchi
Copy link
Contributor

AndreaRicchi commented Dec 23, 2021

@nandlab

I checked your code, that was the same situation as me. As I previously said I temporarily resolved the issue by moving the

#include "mqtt_client_cpp.hpp"

    typedef std::shared_ptr<
        mqtt::callable_overlay<
            mqtt::async_client<
                mqtt::tcp_endpoint<
                    mqtt::as::ip::tcp::socket,
                    mqtt::strand
                >
            >
        >
    >
    MQTTAsyncClient_t;

in the CPP file as a static variable. This improved enormously the compile time.

@BrettHemes
Copy link

This got me bad too... under the pattern of having an MQTT client as a class member I very quickly had my compile times exceed 5-10 minutes for a relatively simple project. By making the client a static variable as suggested, my compile time is back to around 15 seconds.

I have enjoyed developing with this library but do not see this workaround as an acceptable long term solution.

Any updates on future fixes?

@redboltz
Copy link
Owner

redboltz commented Feb 22, 2022

I am working on fixing delivery guarantee issue and reviewing authentication and authorization.
Unfortunately, no significant updates for this issue so far.

NOTE:
mqtt_cpp design principles are header-only and value semantics (as long as it can be used)
See https://wandbox.org/permlink/Rh1X8yI8DMAEOqzU

@redboltz
Copy link
Owner

I analized compile process using -ftime-trace.
It seems that non-template inline functions are always processed.
So I templatized them to avoid instanciate.
Off course actually used functions are instantiate but unused functions (especially many client factory functions) are not instantiated.
Try #931

@ineffective
Copy link

I solved my problems with long compilation times by creating thin, non-templated wrapper for mqtt-client that I use throughout the code. mqtt-client itself is created in separate cpp file that provides factory method. Works like a charm.

@redboltz
Copy link
Owner

@ineffective could you show me your code? I'm interested in which part do you wrap. Only factory functions, or class template client, async_client, sync_client, and endpoint.

@ineffective
Copy link

It is very simple, really. I can't you show you exact code as it is owned by a company I work for, but the idea is as follows:

I created endpoint_sink interface more or less like this:

class endpoint_sink : public enable_shared_from_this<endpoint_sink> {
public:
    virtual void send(UUID sender, UUID receiver, MSG message);
    // ... destructor and few other members, but not important
};

and then in other header:

// MQTT client factory
std::shared_ptr<endpoint_sink> make_mqtt_client (ROUTER router, CONNDATA conndata);

and in implementation file, endpoint_mqtt.cpp:

#include <mqtt_client_cpp.hpp>

template <typename T>
class mqtt_client : public virtual endoint_sink, shared_from_this<mqtt_client> {
public:
    mqtt_client(std::shared_ptr<T> client, ROUTER router,  ...other params) : ... { }
};

// implement factory function:
std::shared_ptr<endpoint_sink> make_mqtt_client(ROUTER router, CONNDATA conndata) {
    auto client = MQTT_NS::make_tls_sync_client(router.get_io_context(), ....);
    using client_type = mqtt_client<std::remove_reference_t<decltype(*client)>>;

    return std::make_shared<client_type>(client, router, ...rest);
}

In the code above:

  • UUID - endpoints in the system are identified using regular uuids
  • ROUTER - component responsible for routing messages, depending on UUID message can be sent further via mqtt or routed locally, through unix socket or (rarely) TCP/IP connection.
  • CONNDATA is regular stuff like server address, certificate and key files, connection timeout and so on.

I also omitted all additional qualifiers on parameters (CV, reference, pointer and so on) as they add nothing of importance and overshadow main points.

Since changes to this file are basically none, it is compiled once and sits there untouched for most of development time. The only problem (but mild) is when source code is pushed to our gitlab and Jenkins starts building it, but even then it still builds much faster than our Angular-based projects.

Also: reason for using sync_client instead of async_client are very simple and somewhat embarrassing: when I wrote this code I had only rudimentary knowledge about boost::asio and using sync_client was easier.

Please let me know if you want to know anything else.

@redboltz
Copy link
Owner

@ineffective , thank you very much!

I'm still trying to improve compilation time in generic way. That means if user defined something like MQTT_BUILD_LIB then build static or shared library like libmqtt_cpp.a.
I achieved factory functions like make_async_client with boost::asio::io_context version.
The library is successfully created. On factory functions, no template instantiation happens. But it is not a big impact for compilation time. Because member functions are template. e.g. async_publish(). It still instantiate.
Actually, the library version takes longer time including linking pre-build library.

So I wanted to know your way. If you use only fixed type async_publish() like member function template and wrap it in cpp file, then compilation time would be improved. That's my guessing.
And it seems that your did that.

However, it cannot be a generic solution but it is good solution for your application.

I expected you might find something miracle way ;)

@ineffective
Copy link

Honestly I don't think compilation times are that bad, I am using boost::asio a lot and it takes takes quite a long time to compile, but works very, very fast, so trade off is absolutely worth it. And all of this is nothing when compared to boost::beast which I also use in few places, compilation times of that library are on a completely different level when compared with boost::asio and mqtt_cpp. All in all I think that while my approach cannot be used on library level, it can be easily adopted by anyone who knows what his/her requirements are. I don't know if it is worth describing it in some official documentation, but maybe hinting at it in some FAQ document would help others to bring compilation times down.

Of course if someone only tries to find out what can be done using your library and writes only simple things in simple file, long compilation times can be off-putting. But I strongly believe that in the end it is worth it.

@redboltz
Copy link
Owner

@ineffective , thanks you.
I think that the document is useful. I prepared the place of the document on the wiki.

https://github.com/redboltz/mqtt_cpp/wiki/ruduce-compilation-time

I believe that you have write permission. Could you write there ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants