ui: Compile boost::signals2 only once #13634

maflcko · 2018-07-11T13:36:18Z

ui is one of the modules that poison other modules with boost/signals2 headers. This moves the include to the cpp file and uses a forward declaration in the header.

Locally this speeds up the incremental build (building everything that uses the ui module) with gcc by ~5% for me. Gcc uses ~5% less memory.

Would be nice if someone could verify the numbers roughly.

I presume the improvements will be more pronounced if the other models would stop exposing the boost header as well.

laanwj · 2018-07-11T13:56:09Z

Concept ACK.

Though, I wonder if we do this, it would make sense to move this away from boost::signals completely for the UI interface (registering interface objects as delegates instead) instead of working to hide boost::signals. But no strong opinion on that.

practicalswift · 2018-07-12T14:08:11Z

Concept ACK

maflcko · 2018-07-19T21:18:20Z

This seemed like a straightforward refactor with a clear win. Not sure if the "registering interface objects as delegates instead" is as simple as that.

DrahtBot · 2018-08-03T20:14:12Z

Note to reviewers: This pull request conflicts with the following ones:

Refactor: separate wallet from node #10973 (Refactor: separate wallet from node by ryanofsky)
Multiprocess bitcoin #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

kallewoof

utACK fa15f97

kallewoof · 2018-08-07T04:35:11Z

src/ui_interface.h

@@ -72,43 +75,69 @@ class CClientUIInterface
        MSG_ERROR = (ICON_ERROR | BTN_OK | MODAL)
    };

+#define ADD_SIGNALS_DECL_WRAPPER(signal_name)                                              \
+    boost::signals2::connection signal_name##_connect(std::function<signal_name##Sig> fn); \
+    void signal_name##_disconnect(std::function<signal_name##Sig> fn);


OCD-nit: remove ending ; to avoid ;; when used

kallewoof · 2018-08-07T04:38:45Z

src/ui_interface.h

-    boost::signals2::signal<bool (const std::string& message, const std::string& caption, unsigned int style), boost::signals2::last_value<bool> > ThreadSafeMessageBox;
+    bool  ThreadSafeMessageBox          (const std::string& message, const std::string& caption, unsigned int style);
+    using ThreadSafeMessageBoxSig = bool(const std::string& message, const std::string& caption, unsigned int style);
+    ADD_SIGNALS_DECL_WRAPPER(ThreadSafeMessageBox);


Maybe overkill but

#define ADD_SIGNALS_DECL_WRAPPER(signal_name, rtype, args...) \ rtype signal_name(args);\ using signal_name##Sig = rtype(args);\ [...]

then e.g.

ADD_SIGNALS_DECL_WRAPPER(ThreadSafeMessageBox, bool, const std::string& message, const std::string& caption, unsigned int style);

Oh thanks! That makes the diff in the header file look a lot cleaner.

kallewoof

re-utACK fa5ce27

ken2812221 · 2018-08-08T02:59:05Z

utACK fa5ce27

ddddce0 util: Replace boost::signals2 with std::function (MarcoFalke) Pull request description: This removes the `#include <boost/signals2/signal.hpp>` from `util.h` (hopefully speeding up the build time and reducing the memory usage further after #13634) The whole translation interface is replaced by a function `G_TRANSLATION_FUN` that is set to nullptr in units that don't need translation. (Thus only set in the gui) Tree-SHA512: 087c717358bbed8bdb409463e225239d667f1ced381abb10e7cd31a41dcdd2cebe20b43c2ee86f0f8e55d53301f75e963f07421a99a7ff4c0cad2c6a375c5ab1

fa5ce27 ui: Compile boost:signals2 only once (MarcoFalke) Pull request description: ui is one of the modules that poison other modules with `boost/signals2` headers. This moves the include to the cpp file and uses a forward declaration in the header. Locally this speeds up the incremental build (building everything that uses the ui module) with gcc by ~5% for me. Gcc uses ~5% less memory. Would be nice if someone could verify the numbers roughly. I presume the improvements will be more pronounced if the other models would stop exposing the boost header as well. Tree-SHA512: 078360eba330ddbca4268bd8552927eae242a239e18dfded25ec20be72650a68cd83af7ac160690249b943d33ae35d15df1313f1f60a0c28b9526853aa7d1e40 # Conflicts: # src/interfaces/node.cpp # src/noui.cpp # src/ui_interface.h

ddddce0 util: Replace boost::signals2 with std::function (MarcoFalke) Pull request description: This removes the `#include <boost/signals2/signal.hpp>` from `util.h` (hopefully speeding up the build time and reducing the memory usage further after bitcoin#13634) The whole translation interface is replaced by a function `G_TRANSLATION_FUN` that is set to nullptr in units that don't need translation. (Thus only set in the gui) Tree-SHA512: 087c717358bbed8bdb409463e225239d667f1ced381abb10e7cd31a41dcdd2cebe20b43c2ee86f0f8e55d53301f75e963f07421a99a7ff4c0cad2c6a375c5ab1 # Conflicts: # src/bench/bench_dash.cpp # src/qt/dash.cpp # src/qt/splashscreen.cpp # src/qt/transactiontablemodel.cpp # src/test/test_dash.cpp # src/util/system.h # src/wallet/coinselection.cpp