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

util: Replace boost::signals2 with std::function #13961

Merged
merged 1 commit into from Aug 25, 2018

Conversation

Projects
None yet
6 participants
@MarcoFalke
Copy link
Member

commented Aug 13, 2018

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)

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1808-utilFun branch 4 times, most recently Aug 13, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2018

For all bitcoind units that depend on util:

  • compile time from 1:13 to 0:42
  • compile mem from 407Mb to 389Mb

(gcc default ./configure)

@practicalswift

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

Excellent!

Concept ACK

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2018

Note to reviewers: This pull request conflicts with the following ones:
  • #14053 (Add address-based index (attempt 4?) by marcinja)
  • #13389 (Utils and libraries: Fix #13371 - move umask operation earlier in AppInit() by n2yen)
  • #13222 (test: Improve test coverage of SelectCoinsBnB by Empact)

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.

@Empact

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

utACK fa7db59

src/util.h Outdated
extern const char * const BITCOIN_CONF_FILENAME;
extern const char * const BITCOIN_PID_FILENAME;

/** Translate a message to the native language of the user. */
const extern std::function<std::string(const char*)>* G_TRANSLATION_FUN;

This comment has been minimized.

Copy link
@promag

promag Aug 23, 2018

Member

How about

const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;

And below (L51) use https://en.cppreference.com/w/cpp/utility/functional/function/operator_bool?

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Aug 24, 2018

Author Member

Done

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1808-utilFun branch Aug 24, 2018

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1808-utilFun branch to ddddce0 Aug 24, 2018

@laanwj

This comment has been minimized.

Copy link
Member

commented Aug 25, 2018

utACK ddddce0

@laanwj laanwj merged commit ddddce0 into bitcoin:master Aug 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Aug 25, 2018

Merge #13961: util: Replace boost::signals2 with std::function
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

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1808-utilFun branch Aug 25, 2018

@@ -13,6 +13,8 @@

#include <memory>

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;

This comment has been minimized.

Copy link
@promag

promag Aug 27, 2018

Member

These = nullptr are not needed since it's not a pointer and the empty constructor has the same behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.