Skip to content

Conversation

fanquake
Copy link
Member

We don't use the serialization or archiving facilities of multi_index.
So globally disable support, which gives a minor improvement in build
time, i.e less preprocessing work, given we don't link any Boost libs.

See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html

Serialization capabilities are automatically provided by just linking with the appropriate Boost.Serialization library module: it is not necessary to explicitly include any header from Boost.Serialization, apart from those declaring the type of archive used in the process. If not used, however, serialization support can be disabled by globally defining the macro BOOST_MULTI_INDEX_DISABLE_SERIALIZATION. Disabling serialization for Boost.MultiIndex can yield a small improvement in build times, and may be necessary in those defective compilers that fail to correctly process Boost.Serialization headers.

@hebasto
Copy link
Member

hebasto commented Mar 14, 2022

Concept ACK on being explicit about used features.

@luke-jr
Copy link
Member

luke-jr commented Mar 21, 2022

I haven't the foggiest clue why, but this breaks the build entirely for me (for both GCC and Clang):

In file included from /usr/lib/gcc/powerpc64le-unknown-linux-gnu/11.2.0/include/g++-v11/ext/string_conversions.h:41,
                 from /usr/lib/gcc/powerpc64le-unknown-linux-gnu/11.2.0/include/g++-v11/bits/basic_string.h:6607,
                 from /usr/lib/gcc/powerpc64le-unknown-linux-gnu/11.2.0/include/g++-v11/string:55,
                 from ./chainparamsbase.h:9,
                 from ./chainparams.h:9,
                 from ./net.h:9,
                 from ./net_processing.h:9,
                 from rpc/server_util.cpp:7:
/usr/lib/gcc/powerpc64le-unknown-linux-gnu/11.2.0/include/g++-v11/cstdlib:75:15: fatal error: stdlib.h: No such file or directory
   75 | #include_next <stdlib.h>
      |               ^~~~~~~~~~
compilation terminated.

@luke-jr
Copy link
Member

luke-jr commented Mar 21, 2022

Upon further research, this is a bug in 0.21 up to and including master. Fixing in #24633

@fanquake
Copy link
Member Author

Rebased now that #24633 has been merged.

We don't use the serialilzation or archiving facilities of multi_index.
So globally disable support, which gives a minor improvement in build
time, i.e less preprocessing work.

See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html
@fanquake fanquake force-pushed the no_boost_multi_index_serialization branch from 785c9e2 to 0d01272 Compare April 2, 2022 14:47
@fanquake
Copy link
Member Author

fanquake commented Apr 2, 2022

@mzumsande when you were discussing a new serialization format, I assume you weren't planning on using Boost Multi Index Serialization?

@fanquake
Copy link
Member Author

fanquake commented Apr 2, 2022

Guix Build (on x86_64):

7a16c153e5533f0737f995f2718d55e8954f07203427c963556f1c4492614bd6  guix-build-0d01272cd883/output/aarch64-linux-gnu/SHA256SUMS.part
6144bc6b9a1aeda19c9e4d4c8c163ae77b99786b04cdf81ea111eded1e325fcb  guix-build-0d01272cd883/output/aarch64-linux-gnu/bitcoin-0d01272cd883-aarch64-linux-gnu-debug.tar.gz
a1c581bbbcc2839d432618f5f1feaf44409b8216e244bda9e2ef596b8161b98b  guix-build-0d01272cd883/output/aarch64-linux-gnu/bitcoin-0d01272cd883-aarch64-linux-gnu.tar.gz
77ef7a594e6d425aa7655ba02b830549740c09f7fe847803a90c02234155d939  guix-build-0d01272cd883/output/arm-linux-gnueabihf/SHA256SUMS.part
b6a7d79ebc588f4d324e4e9ccf83aab204bb6850b88502a4728266e1798777cc  guix-build-0d01272cd883/output/arm-linux-gnueabihf/bitcoin-0d01272cd883-arm-linux-gnueabihf-debug.tar.gz
d3afc694220dfd291da3fb7524e7381a2bda4a84ccd6e2d32b9cf19ce4ccb67a  guix-build-0d01272cd883/output/arm-linux-gnueabihf/bitcoin-0d01272cd883-arm-linux-gnueabihf.tar.gz
ba22df86190d303767b3b2dae3f8e60a9cea3a6d90ea4a92cf2fdf05fd65e655  guix-build-0d01272cd883/output/arm64-apple-darwin/SHA256SUMS.part
4e7077078dcd551e843f258a6432650ca075db5e8cb9e04b44bd2df56f2df9c3  guix-build-0d01272cd883/output/arm64-apple-darwin/bitcoin-0d01272cd883-arm64-apple-darwin-unsigned.dmg
e52bdfbf8a3b7dcc539a85199aadf02565aa38abad98b3bd3ff2db4c0f325ac9  guix-build-0d01272cd883/output/arm64-apple-darwin/bitcoin-0d01272cd883-arm64-apple-darwin-unsigned.tar.gz
56a11d3ee4c5bd36209503733310df1d5205d6b385e7163f5394a01fdaefd298  guix-build-0d01272cd883/output/arm64-apple-darwin/bitcoin-0d01272cd883-arm64-apple-darwin.tar.gz
6f91fc8cc9d72d69fddf400c311bb9a2468a582931a9f7914d32fe2b2c38cce8  guix-build-0d01272cd883/output/dist-archive/bitcoin-0d01272cd883.tar.gz
5361844bd2e6a44a7e90ba3a0577f92d602d440c85463c2550d5ca6be10df94c  guix-build-0d01272cd883/output/powerpc64-linux-gnu/SHA256SUMS.part
dcc31626364a659d5c44bb5ebb6d8ff3ac148efc5aa9b26af35738b81f71b75a  guix-build-0d01272cd883/output/powerpc64-linux-gnu/bitcoin-0d01272cd883-powerpc64-linux-gnu-debug.tar.gz
c48e5a063cbe951c46bd9ebe8b05ca906e48e90d86bd73edb49a7e0cef603dae  guix-build-0d01272cd883/output/powerpc64-linux-gnu/bitcoin-0d01272cd883-powerpc64-linux-gnu.tar.gz
f590457930d4344eb6383288790508ba868ed3fe2ad3de1b77cc2f31c8b038cf  guix-build-0d01272cd883/output/powerpc64le-linux-gnu/SHA256SUMS.part
ae3bb94835bc962594657685efad948dd0e13a780664f52cfc7ce47476e45a77  guix-build-0d01272cd883/output/powerpc64le-linux-gnu/bitcoin-0d01272cd883-powerpc64le-linux-gnu-debug.tar.gz
2b8549f911ffaacfc26ca9baa23cec342283e5b8b2fb0152318208dfc0e7bcb2  guix-build-0d01272cd883/output/powerpc64le-linux-gnu/bitcoin-0d01272cd883-powerpc64le-linux-gnu.tar.gz
3c0a0d7ca04a6c06b7b645d9191ff633a8c32c81e621b152958f4b700646a083  guix-build-0d01272cd883/output/riscv64-linux-gnu/SHA256SUMS.part
ca8965ec9ee5730bece34ae64b2d5add0cf4c9f99cde51f89d9fd41033ecb27f  guix-build-0d01272cd883/output/riscv64-linux-gnu/bitcoin-0d01272cd883-riscv64-linux-gnu-debug.tar.gz
6fc0c313afddacaa3261b9282b4bc8c1ca9f83ebe77e49706bffa1e9ba716f09  guix-build-0d01272cd883/output/riscv64-linux-gnu/bitcoin-0d01272cd883-riscv64-linux-gnu.tar.gz
1825d8710c75e059195112b45248a1ed0558bd43a78f3ba1f62d3c6ac5aff545  guix-build-0d01272cd883/output/x86_64-apple-darwin/SHA256SUMS.part
d8f957f34aa88a32741ea9965fc0fd9123e62aecfefa700206bf6a360c147508  guix-build-0d01272cd883/output/x86_64-apple-darwin/bitcoin-0d01272cd883-x86_64-apple-darwin-unsigned.dmg
b6a3633f336c4574c1d0456a26aae2855ab876c6af7ff458c79a2dbb098bb0b4  guix-build-0d01272cd883/output/x86_64-apple-darwin/bitcoin-0d01272cd883-x86_64-apple-darwin-unsigned.tar.gz
5d2291d55f53979adc1e44bfb44d6234764dca7416eb3f64179abab5cbdf6371  guix-build-0d01272cd883/output/x86_64-apple-darwin/bitcoin-0d01272cd883-x86_64-apple-darwin.tar.gz
c6d99e81a474f26e0b468c2aa22df30427c1478f14f0632910e188d9034b477e  guix-build-0d01272cd883/output/x86_64-linux-gnu/SHA256SUMS.part
b6812bfad53f4173b9f4ca88ea2c87c975d13cbf5040f9abef06889ffb3e0632  guix-build-0d01272cd883/output/x86_64-linux-gnu/bitcoin-0d01272cd883-x86_64-linux-gnu-debug.tar.gz
f8697d830aecfdbdbcfbafb0387876d5b3429a069f25491069d7c3418e5c7b6d  guix-build-0d01272cd883/output/x86_64-linux-gnu/bitcoin-0d01272cd883-x86_64-linux-gnu.tar.gz
f8a43427631cdfda0a0f81e1452647e489edce0250ab73cf147ebbf8865d7aa8  guix-build-0d01272cd883/output/x86_64-w64-mingw32/SHA256SUMS.part
49e7b3033ea0df302a3373de1494990716a7e23226154739da822df3ff384f63  guix-build-0d01272cd883/output/x86_64-w64-mingw32/bitcoin-0d01272cd883-win64-debug.zip
7455c6670aef26828d775cc2575dac12e4bfb4e5814be7823cdeed4b5a51cdba  guix-build-0d01272cd883/output/x86_64-w64-mingw32/bitcoin-0d01272cd883-win64-setup-unsigned.exe
7ea15de031048fc86f3a3c9ba1b351b1d05926690f774444862d3284dcd8d2a3  guix-build-0d01272cd883/output/x86_64-w64-mingw32/bitcoin-0d01272cd883-win64-unsigned.tar.gz
1e0bfadfb0d5cffb56a35e773c58584c25ddb721d42fa2c7e2287e5d175c6d5e  guix-build-0d01272cd883/output/x86_64-w64-mingw32/bitcoin-0d01272cd883-win64.zip

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 3, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24742 ([POC] build: prune Boost headers in depends by fanquake)

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.

@mzumsande
Copy link
Contributor

@mzumsande when you were discussing a new serialization format, I assume you weren't planning on using Boost Multi Index Serialization?

no such plans, Concept ACK on disabling it.

@maflcko
Copy link
Member

maflcko commented Apr 4, 2022

No objections, but I don't get a speedup locally.

@fanquake
Copy link
Member Author

fanquake commented Apr 4, 2022

but I don't get a speedup locally.

The speedup is basically a rounding error, but exists in the reduced amount of preprocessing work. i.e:

// txmempool.h
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/ordered_index.hpp>
#include <boost/multi_index/sequenced_index.hpp>
#include <boost/multi_index_container.hpp>

int main() {
	return 0;
}
time clang++ -E boost.cpp | wc -l
   98416

vs

time clang++ -E boost.cpp -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION | wc -l
   94947

The main goal is to explicitly opt-out of things we don't use.

@maflcko
Copy link
Member

maflcko commented Apr 4, 2022

cr ACK 0d01272

@maflcko maflcko merged commit 62efdfb into bitcoin:master Apr 4, 2022
@fanquake fanquake deleted the no_boost_multi_index_serialization branch April 4, 2022 07:59
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 4, 2022
…alization

0d01272 build: don't use Boost multi_index serialization (fanquake)

Pull request description:

  We don't use the serialization or archiving facilities of multi_index.
  So globally disable support, which gives a minor improvement in build
  time, i.e less preprocessing work, given we don't link any Boost libs.

  See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html

  > Serialization capabilities are automatically provided by just linking with the appropriate Boost.Serialization library module: it is not necessary to explicitly include any header from Boost.Serialization, apart from those declaring the type of archive used in the process. If not used, however, serialization support can be disabled by globally defining the macro BOOST_MULTI_INDEX_DISABLE_SERIALIZATION. Disabling serialization for Boost.MultiIndex can yield a small improvement in build times, and may be necessary in those defective compilers that fail to correctly process Boost.Serialization headers.

ACKs for top commit:
  MarcoFalke:
    cr ACK 0d01272

Tree-SHA512: 87c664a2f142dc6b8f8598341f9829be3fda8cf614d73cc9a894c8033ee40c6daa9b50f4049ecb1f1e3aaf342568d9a5f5c65af1e04c36ee3a9cb46eca95767b
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request May 21, 2022
We don't use the serialilzation or archiving facilities of multi_index.
So globally disable support, which gives a minor improvement in build
time, i.e less preprocessing work.

See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html

Github-Pull: bitcoin#24558
Rebased-From: 4944175
@bitcoin bitcoin locked and limited conversation to collaborators Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants