tests: Reduce memory usage by 0.5 GB when compiling script

practicalswift commented at 2:16 pm on June 29, 2019: contributor

Reduce compile-time memory usage (max RSS) by 0.5 GB when compiling script_tests.cpp (from 1.4 GB to 0.9 GB).

Context:

See compile-time memory consumption wall of shame in this month’s State of the Repo Report.
TheBlueMatt’s issue #6658: “Building requires >1GB memory”

Compile-time memory consumption wall of shame before this PR:

#	File	Max memory usage (RSS)
1	`test/test_test_bitcoin-script_tests.o`	1416 MB
2	`wallet/libbitcoin_wallet_a-wallet.o`	1102 MB
3	`wallet/libbitcoin_wallet_a-rpcwallet.o`	1059 MB
4	`libbitcoin_server_a-validation.o`	953 MB
5	`test/test_test_bitcoin-util_tests.o`	905 MB

Compile-time memory consumption wall of shame after this PR:

#	File	Max memory usage (RSS)
1	`wallet/libbitcoin_wallet_a-wallet.o`	1102 MB
2	`wallet/libbitcoin_wallet_a-rpcwallet.o`	1059 MB
3	`libbitcoin_server_a-validation.o`	953 MB
4	`test/test_test_bitcoin-script_tests.o`	948 MB
5	`test/test_test_bitcoin-util_tests.o`	905 MB

Before:

0$ /usr/bin/time g++ -std=c++11 -DHAVE_CONFIG_H -I. -I../src/config -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I. -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/usr/include -I./leveldb/include -I./leveldb/helpers/memenv -I./secp256k1/include -I./univalue/include -DBOOST_TEST_DYN_LINK -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -fstack-reuse=none -Wstack-protector -fstack-protector-all -Wall -Wextra -Wformat -Wvla -Wformat-security -Wredundant-decls -Wno-unused-parameter -Wno-implicit-fallthrough -fPIE -g -O2 -MT test/test_test_bitcoin-script_tests.o -MD -MP -MF test/.deps/test_test_bitcoin-script_tests.Tpo -c -o test/test_test_bitcoin-script_tests.o test/script_tests.cpp
132.63user 0.99system 0:33.63elapsed 99%CPU (0avgtext+0avgdata 1443460maxresident)k
20inputs+114672outputs (0major+439941minor)pagefaults 0swaps

After:

0$ /usr/bin/time g++ -std=c++11 -DHAVE_CONFIG_H -I. -I../src/config -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I. -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/usr/include -I./leveldb/include -I./leveldb/helpers/memenv -I./secp256k1/include -I./univalue/include -DBOOST_TEST_DYN_LINK -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -fstack-reuse=none -Wstack-protector -fstack-protector-all -Wall -Wextra -Wformat -Wvla -Wformat-security -Wredundant-decls -Wno-unused-parameter -Wno-implicit-fallthrough -fPIE -g -O2 -MT test/test_test_bitcoin-script_tests.o -MD -MP -MF test/.deps/test_test_bitcoin-script_tests.Tpo -c -o test/test_test_bitcoin-script_tests.o test/script_tests.cpp
128.86user 0.95system 0:36.14elapsed 82%CPU (0avgtext+0avgdata 947984maxresident)k
20inputs+66112outputs (0major+285149minor)pagefaults 0swaps

fanquake added the label Tests on Jun 29, 2019

practicalswift force-pushed on Jun 29, 2019

Sjors commented at 4:27 pm on June 29, 2019: member

Concept ACK, but I get three warnings on macOS:

 0test/script_tests.cpp:489:13: warning: unknown pragma ignored [-Wunknown-pragmas]
 1#pragma GCC push_options
 2            ^
 3test/script_tests.cpp:490:13: warning: unknown pragma ignored [-Wunknown-pragmas]
 4#pragma GCC optimize ("O0")
 5            ^
 6test/script_tests.cpp:969:13: warning: unknown pragma ignored [-Wunknown-pragmas]
 7#pragma GCC pop_options
 8            ^
 93 warnings generated.
10  CXXLD    test/test_bitcoin

We should get rid of those warnings. However I assume this change won’t do anything on macOS: that’s probably fine, since most low memory devices don’t run macOS.

fanquake renamed this:
~~tests: Reduce compile-time memory usage by 0.5 GB when compiling script_tests.cpp (from 1.4 GB to 0.9 GB)~~
tests: Reduce memory usage when compiling script_tests.cpp
on Jun 30, 2019

practicalswift renamed this:
~~tests: Reduce memory usage when compiling script_tests.cpp~~
tests: Reduce memory usage by 0.5 GB when compiling script_tests.cpp
on Jun 30, 2019

practicalswift commented at 7:35 am on June 30, 2019: contributor

~@Sjors Are you sure you are testing against the latest version (e87b991b5de7f98d3b260c44b97f154bd40a386b) which has the ifdef? If so, what compiler are you using?~ @Sjors Thanks for testing! Please try the latest version - the unknown pragma warning should be addressed :-) @fanquake The 0.5 GB part is highly relevant for reviewer interest and should be in the title, no?

promag commented at 9:17 am on June 30, 2019: member

Tests should be compiled the same way - same optimizations? Because of inlined code and templates?

practicalswift commented at 9:21 am on June 30, 2019: contributor

@practicalswift Ideally yes, but at the cost of a +0.5 GB memory requirement? :-)

promag commented at 9:30 am on June 30, 2019: member

IMHO yes.

practicalswift commented at 4:07 pm on June 30, 2019: contributor

@promag Oh, that is surprising. Care to elaborate? What potentials problems do you see? I’m not sure I follow your reasoning here TBH :-)

promag commented at 4:17 pm on June 30, 2019: member

I’m not saying there are problems, just that it should use the same compiler flags.

Would breaking the file help?

BTW, is the test slower?

practicalswift commented at 7:54 pm on June 30, 2019: contributor

@promag I still don’t follow. It seemed like you saw some potential problems with this PR “because of inlined code and templates”? I fail to see those problems so that’s why I need your clarification - otherwise it is very hard to address your concerns properly :-)

Disable gcc optimisations for script_build test to achieve sane compile-time memory usage d522fdd901

practicalswift force-pushed on Jun 30, 2019

promag commented at 8:17 pm on June 30, 2019: member

Generically speaking inlined code and templates will be compiled and tested differently correct?

practicalswift commented at 8:51 pm on June 30, 2019: contributor

Would breaking the file help?

Unfortunately not: max RSS when compiling only script_build is 1.2 GB.

BTW, is the test slower?

There is no measurable difference: the run-time of script_tests/script_build is ~380 ms regardless of optimisation level.

laanwj commented at 11:42 am on July 2, 2019: member

Concept ACK

Sjors commented at 2:51 pm on July 2, 2019: member

@promag afaik the compiler flags used for the test have no impact on the binary code produced for the file that’s being tested. If that’s the case, then there’s no reason to believe that these new tests would miss any problems. @practicalswift the warnings have gone away

ACK d522fdd, but that only covers not breaking macOS.

MarcoFalke commented at 2:58 pm on July 2, 2019: member

I am not too happy about pushing and popping compiler arguments on a per-file basis.

Also, I’d first like to understand why it is using so much memory. Clang 9.0 hast -ftime-trace, maybe gcc has something similar.

promag commented at 3:11 pm on July 2, 2019: member

@Sjors I know that, see https://stackoverflow.com/questions/6364365/can-different-optimization-levels-lead-to-functionally-different-code, and considering the test file in question why risk some weird case?

Agree with @MarcoFalke regarding other approaches to reduce mem usage.

practicalswift commented at 11:24 am on July 3, 2019: contributor

@MarcoFalke The reason for the absurd memory use is -fvar-tracking (in combination with -g). See #16331 for alternative way to handle this.

practicalswift closed this on Jul 30, 2019

practicalswift deleted the branch on Apr 10, 2021

DrahtBot locked this on Aug 18, 2022

tests: Reduce memory usage by 0.5 GB when compiling script_tests.cpp #16312