9ad2fe7e69 clusterlin: only start/use search when enough iterations left (Pieter Wuille)
bd044356ed clusterlin: improve heuristic to decide split transaction (optimization) (Pieter Wuille)
71f2629398 clusterlin: include topological pot subsets automatically (optimization) (Pieter Wuille)
e20fda77a2 clusterlin: reduce computation of unnecessary pot sets (optimization) (Pieter Wuille)
6060a948ca clusterlin bench: add example hard cluster benchmarks (Pieter Wuille)
2965fbf203 clusterlin: track upper bound potential set for work items (optimization) (Pieter Wuille)
9e43e4ce10 clusterlin: use feerate-sorted depgraph in SearchCandidateFinder (Pieter Wuille)
b80e6dfe78 clusterlin: add reordering support for DepGraph (Pieter Wuille)
85a285a306 clusterlin: separate initial search entries per component (optimization) (Pieter Wuille)
e4faea9ca7 clusterlin bench: have low/high iter benchmarks instead of per-iter (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289
Depends on #30126, and was split off from it.
This improves the candidate search algorithm introduced in the previous PR with a variety of optimizations.
The resulting search algorithm largely follows Section 2 of [How to linearize your cluster](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-2-finding-high-feerate-subsets-5), though with a few changes:
* Connected component analysis is performed inside the search algorithm (creating initial work items per component for each candidate), rather than once at a higher level. This duplicates some work but is significantly simpler in implementation.
* No ancestor-set based presplitting inside the search is performed; instead, the `best` value is initialized with the best topologically valid set known to the LIMO algorithm before search starts: the better one out of the highest-feerate remaining ancestor set, and the highest-feerate prefix of remaining transactions in `old_linearization`.
* Work items are represented using an included set *inc* and an undefined set *und*, rather than included and excluded.
* Potential sets *pot* are not computed for work items with empty *inc*.
At a high level, the only missing optimization from that post is bottleneck analysis; my thinking is that it only really helps with clusters that are already relatively cheap to linearize (doing so would need to be done at a higher level, not inside the search algorithm).
---
Overview of the impact of each commit here on linearize performance:
* **[clusterlin bench: have low/high iter benchmarks instead of per-iter](21a184db63)**: no impact
* **[separate initial search entries per component (optimization)](c84c5c86ba)**: reduce iterations, increase start-up cost
* **[add reordering support for DepGraph](019ff29609)**: no impact
* **[use feerate-sorted depgraph in SearchCandidateFinder](8e27dd5a22)**: typically reduce iterations, increase start-up cost
* **[track upper bound potential set for work items](781e0fb3aa)**: reduce iterations, increase cost per iteration
* **[reduce computation of unnecessary pot sets](9fe834fa97)**: reduce cost per iteration
* **[include topological pot subsets automatically](30612710a4)**: reduce iterations, increase cost per iteration
* **[improve heuristic to decide split transaction](1880c00ab1)**: typically reduce iterations, increase cost per iteration
* **[only start/use search when enough iterations left](12760a57b3)**: just account for start-up cost as equivalent iterations
ACKs for top commit:
sdaftuar:
ACK 9ad2fe7e69
instagibbs:
reACK 9ad2fe7e69
glozow:
reACK 9ad2fe7e69, just have a question about the docs
Tree-SHA512: 108bcbb0676f36071eb83954059b5f3d6646c745015b644a2a5d7f5a8ac9424c2d01d339fa6318a3aff4cf313308e85bb80b0090899720a3fcba027b8025590a
bc7900f33d kernel: Move background load thread to node context (TheCharlatan)
Pull request description:
The thread handle is never used by the ChainstateManager, so move it out and into the node context. Users of the kernel library now no longer have to manually join the thread when destructing the ChainstateManager.
ACKs for top commit:
maflcko:
ACK bc7900f33d🔄
achow101:
ACK bc7900f33d
ryanofsky:
Code review ACK bc7900f33d. Nice cleanup
jonatack:
Light ACK bc7900f33d
stickies-v:
ACK bc7900f33d
Tree-SHA512: add9c4823731324e3db50f95e023e99d55db7cc75c69083ae7c9c2157e5540968caa6cf10674aa4901f91366b02ebb1ff18bb977fec0a46431e2196448958b9d
72c9a1fe94 test: Check that network stays suspended after dumptxoutset if it was off before (Fabian Jahr)
Pull request description:
Follow-up to #30817 which covered the robustness of `dumptxoutset`: network is deactivated during the run but re-activated even when an issue was encountered. But it did not cover the case if the user had deactivated the network themselves before. In that case the user may want the network to stay off so the network is not reactivated after `dumptxoutset` finishes. A test for this behavior is added here.
ACKs for top commit:
achow101:
ACK 72c9a1fe94
pablomartin4btc:
ACK 72c9a1fe94
theStack:
utACK 72c9a1fe94
tdb3:
tested ACK 72c9a1fe94
Tree-SHA512: 18a57c5782e99a018414db0597e88debeb32666712c2a75dddbb55135d8f1ddd1eeacba8bbbd35fc03b6c4ab0522fe074ec08edea729560b018f51efabf00e89
07f4cebe57 refactor: move m_is_inbound out of CNodeState (Sergi Delgado Segura)
Pull request description:
`m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we are currently storing it in `CNodeState`, which requires locking `cs_main` in order to access it. This can be moved to the outside scope and only require `m_peer_mutex`.
This is a refactor in preparation for Erlay reworks.
ACKs for top commit:
maflcko:
ACK 07f4cebe57 🗞
achow101:
ACK 07f4cebe57
marcofleon:
ACK 07f4cebe57
naumenkogs:
ACK 07f4cebe57
Tree-SHA512: bcc77135646c697204a4605971774cb3ccdf11b3e363a7339bfb4d1678de1681d6ca642dc467f7d71834a94dd56e05367e7fd32a3e6dc6be30c89342f39bf695
95560616fb code style: update .editorconfig file (Sebastian Falbesoner)
Pull request description:
Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes:
- consider Rust .rs files (relevant since #28076, commit bbbbdb0c)
- reflect build system change to CMake (#30454, #30664)
- add setting for bare Makefile still used for depends builds
Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). The PR is made under the assumption that the file is still considered useful, especially for new contributors. If people feel like that's not the case anymore, the alternative is to delete it, obviously.
Top commit has no ACKs.
Tree-SHA512: 8406b1caf31e310f7e17c607d97beac583481e71b4425e0be2bbd8207096aa374a70151b58aae5fdb648ef5ff5c7e1d0a2949e6de3355bdd2009d8353ee24af0
282f0e9255 Unit test runner documentation fix and improvements (Jon Atack)
Pull request description:
Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that.
Improve the content and order of the unit test documentation.
ACKs for top commit:
pablomartin4btc:
re-ACK 282f0e9255
tdb3:
re ACK 282f0e9255
Tree-SHA512: 0d25108ab641bcd9b53f99d139afeec90a34f44d5b00c3c677f7539d87782440a28fadc348663b8c28ace43552834737b9c1e8f5517c68edc8547695a9cb5063
Updates the .editorconfig file, first introduced in 2021
(see PR #21123, commit 7a135d57) w.r.t. following changes:
- consider Rust .rs files (relevant since #28076, commit bbbbdb0c)
- reflect build system change to CMake (#30454, #30664)
- add setting for the bare Makefile still used for depends builds
Can be tested e.g. by using the editorconfig-vim plugin
(https://github.com/editorconfig/editorconfig-vim).
- Running `test_bitcoin --help` prints the list of arguments that may be passed,
not the list of tests, so fix that.
- Improve the content and order of the unit test documentation.
001b1cf010 build: use standard branch-protection for aarch64-linux (fanquake)
Pull request description:
Use `-mbranch-protection=standard` when targetting `*aarch64-*-linux*`.
Part of #24123, but this flag can already be used on a best effort basis.
Note that this flag is also already used by default, in the toolchain, on various distros (i.e Fedora).
ACKs for top commit:
hebasto:
ACK 001b1cf010.
TheCharlatan:
ACK 001b1cf010
Tree-SHA512: 2d7ae60f59921a62d51139cb0fd5cecbed4f63266564b2623b7d160f5b0c2c42c78ef8aeff787f485eccc46a9ffd5da70023ec093df6add7c982e0d48a1601b5
The thread handle is never used by the ChainstateManager, so move it out
and into the node context. Users of the kernel library now no longer
have to manually join the thread when destructing the ChainstateManager.
a965f2bc07 gui: fix crash when closing wallet (furszy)
Pull request description:
The crash occurs because `WalletController::removeAndDeleteWallet` is called twice for the
same wallet model: first in the GUI's button connected function `WalletController::closeWallet`,
and then again when the backend emits the `WalletModel::unload` signal.
This causes the issue because `removeAndDeleteWallet` inlines an `erase(std::remove())`.
So, if `std::remove` returns an iterator to the end (indicating the element wasn't found
because it was already erased), the subsequent call to `erase` leads to an undefined behavior.
Test Notes:
Try closing any wallet using the toolbar button in the GUI. It will crash in master, but not here.
Fixes https://github.com/bitcoin/bitcoin/issues/30887.
ACKs for top commit:
pablomartin4btc:
tACK a965f2bc07
jarolrod:
ACK a965f2bc07
hebasto:
ACK a965f2bc07.
Tree-SHA512: c94681b95cb566f7aabd0d4fb10f797c2cea6ac569abc265e918f08e6abae3335432a0b0879372b54b2c109798ed0a4a249bf162c34add59cbd18d38a2d9660e
The crash occurs because 'WalletController::removeAndDeleteWallet' is called
twice for the same wallet model: first in the GUI's button connected function
'WalletController::closeWallet', and then again when the backend emits the
'WalletModel::unload' signal.
This causes the issue because 'removeAndDeleteWallet' inlines an
erase(std::remove()). So, if 'std::remove' returns an iterator to the end
(indicating the element wasn't found because it was already erased), the
subsequent call to 'erase' leads to an undefined behavior.
Empirically, this approach seems to be more efficient in common real-life
clusters, and does not change the worst case.
Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
Automatically add topologically-valid subsets of the potential set pot
to inc. It can be proven that these must be part of the best reachable
topologically-valid set from that work item.
This is a crucial optimization that (apparently) reduces the maximum
number of iterations from ~2^(N-1) to ~sqrt(2^N).
Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
Keep track of which transactions in the graph have an individual
feerate that is better than the best included set so far. Others do not
need to be added to the pot set, as they cannot possibly help beating
best.
In each work item, keep track of a conservative overestimate of the best
possible feerate that can be reached from it, and then use these to avoid
exploring hopeless work items.
Add a DepGraph(depgraph, reordering) function that constructs a new DepGraph
corresponding to an old one, but with its transactions is a modified order
(given as a vector from old to new positions).
Also use this reordering feature inside DepGraphFormatter::Unser, which needs
a small modification so that its reordering mapping is old-to-new (rather than
the new-to-old it used before).
Before this commit, the worst case for linearization involves clusters which
break apart in several smaller components after the first candidate is
included in the output linearization.
Address this by never considering work items that span multiple components
of what remains of the cluster.
19f4a7c95a test: Wait for local services to update in feature_assumeutxo (Fabian Jahr)
Pull request description:
Closes#30878
It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update.
Can be reproduced locally by adding the sleep here:
```cpp
──────────────────────────────────────────────────────────────────────────────────────────────────────────┐
src/validation.cpp:3567: bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< │
──────────────────────────────────────────────────────────────────────────────────────────────────────────┘
}
if (WITH_LOCK(::cs_main, return m_disabled)) {
std::this_thread::sleep_for(std::chrono::seconds(10));
// Background chainstate has reached the snapshot base block, so exit.
// Restart indexes to resume indexing for all blocks unique to the snapshot
```
ACKs for top commit:
maflcko:
review-only ACK 19f4a7c95a
achow101:
ACK 19f4a7c95a
pablomartin4btc:
tACK 19f4a7c95a
furszy:
Code review ACK [19f4a7c](19f4a7c95a).
Tree-SHA512: 70dad3795988956c5e20f2d2d895fb56c5e3ce257c7547d3fd729c88949f0e24cb34594da1537bce8794ad02b2db44e7e46e995aa32539cd4dd84c4f1d4bb1c4
fa5bc450d5 util: Use compile-time check for LogConnectFailure (MarcoFalke)
fa7087b896 util: Use compile-time check for FatalErrorf (MarcoFalke)
faa62c0112 util: Add ConstevalFormatString (MarcoFalke)
fae7b83eb5 lint: Remove forbidden functions from lint-format-strings.py (MarcoFalke)
Pull request description:
The `test/lint/lint-format-strings.py` was designed to count the number of format specifiers and assert that they are equal to the number of parameters passed to the format function. The goal seems reasonable, but the implementation has many problems:
* It is written in Python, meaning that C++ code can not be parsed correctly. Currently it relies on brittle regex and string parsing.
* Apart from the parsing errors, there are also many logic errors. For example, `count_format_specifiers` allows a mix of positional specifiers and non-positional specifiers, which can lead to runtime format bugs. Also, `count_format_specifiers` silently skipped over "special" format specifiers, which are valid in tinyformat, which again can lead to runtime format bugs being undetected.
* The brittle logic has a history of breaking in pull requests that are otherwise fine. This causes the CI to fail and the pull request being blocked from progress until the bug in the linter is fixed, or the code is rewritten to work around the bug.
* It is only run in the CI, or when the developer invokes the script. It would be better if the developer got the error message at compile-time, directly when writing the code.
Fix all issues by using a `consteval` checked format string in `FatalErrorf` and `LogConnectFailure`.
This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places.
ACKs for top commit:
stickies-v:
re-ACK fa5bc450d5
l0rinc:
ACK fa5bc450d5
hodlinator:
ACK fa5bc450d5
ryanofsky:
Code review ACK fa5bc450d5
Tree-SHA512: d6189096b16083143687ed1b1559cf4f92f97dd87bc5d00673e44f4fb9fce7bb7b215cfdfc39b6e6a24f0b75a79a03ededce966639e554f7172e1fc22cf015ae
1eac96a503 Compare FromUserHex result against other hex validators and parsers (Lőrinc)
19947863e1 Use BOOST_CHECK_EQUAL for optional, arith_uint256, uint256, uint160 (Lőrinc)
743ac30e34 Add std::optional support to Boost's equality check (Lőrinc)
Pull request description:
Enhanced `FromUserHex` coverage by:
* Added `std::optional` support to `BOOST_CHECK_EQUAL`, allowing direct comparisons of `std::optional<T>` with other `T` expected values.
* Increased fuzz testing for hex parsing to validate against other hex validators and parsers.
----
* Use BOOST_CHECK_EQUAL for https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706637780 arith_uint256, uint256, uint160
Example error before:
> unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access
test/validation_chainstatemanager_tests.cpp:781: last checkpoint
after:
> test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000]
ACKs for top commit:
stickies-v:
re-ACK 1eac96a503
ryanofsky:
Code review ACK 1eac96a503. Only changes since last review were auto type and fuzz test tweaks.
hodlinator:
ACK 1eac96a503
Tree-SHA512: f1d2c65f0ee4e97830700be5b330189207b11ed0c89a8cebf0f97d43308402a6b3732e10130c79a0c044f7d2eeabfb5359990825aadf02c4ec19428dcd982b00
0dd16d7118 build: Add a pkg-config file for libbitcoinkernel (TheCharlatan)
45be32f838 build: Produce a usable static kernel library (TheCharlatan)
Pull request description:
Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It lacks symbols for the internal libraries, besides those defined in the kernel library target.
Fix this by explicitly installing all the required internal static libraries. To make usage of these installed libraries easy, add a pkg-config file that can be used during linking.
This patch can be tested with:
```
cmake -B build -DBUILD_SHARED_LIBS=OFF -DBUILD_KERNEL_LIB=ON
cmake --build build
cmake --install build
g++ -std=c++20 -o test_chainstate src/bitcoin-chainstate.cpp -I/home/drgrid/bitcoin/src $(pkg-config --libs --static libbitcoinkernel)
```
Attempts to solve #30801
ACKs for top commit:
hebasto:
ACK 0dd16d7118.
fanquake:
ACK 0dd16d7118 - this looks like a good place to start.
ryanofsky:
Code review ACK 0dd16d7118
Tree-SHA512: 92f7bc959584bdc595f4aa6d0ab133355481075fe8564224fd7ac122fd7bdd75f98cf26ef0a6a7d84fd552d2258ddca1b674eca91122469a58bacc5f0a0ec2ef
`m_is_inbound` cannot be changed throughout the life of a `Peer`. However, we
are currently storing it in `CNodeState`, which requires locking `cs_main` in
order to access it. This can be moved to the outside scope and only require
`m_peer_mutex`.
This is a refactor in preparation for Erlay reworks.
72b46f28bf test: fix exclude parsing for functional runner (Max Edwards)
Pull request description:
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded.
PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6`) but it made the wrong assumption that test names intended to be excluded would include the .py extension.
The following https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 shows that this is not how the `--exclude` flag was used in CI.
https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 gave three examples of `--exclude` being used in CI so I compared the number of tests that the runner would run for these three examples in three situations, before #30244 was introduced, in master today and with this PR applied.
Example:
`--previous-releases --coverage --extended --exclude feature_dbcrash`
Test count:
Before #30244 introduced: 314
Master: 315
With this PR: 314
Example:
`--exclude feature_init,rpc_bind,feature_bind_extra`
Test count:
Before #30244 introduced: 306
Master 311
With this PR: 306
Example:
`--exclude rpc_bind,feature_bind_extra`
Before #30244 introduced: 307
Master 311
With this PR: 307
I've also tested that the functionality introduced with #30244 remains and we can still exclude specific tests by argument.
ACKs for top commit:
maflcko:
review ACK 72b46f28bf
willcl-ark:
ACK 72b46f28bf
Tree-SHA512: 37c0e3115f4e3efdf9705f4ff8cd86a5cc906aacc1ab26b0f767f5fb6a953034332b29b0667073f8382a48a2fe9d649b7e60493daf04061260adaa421419d8c8
23eedc5d1e build: Skip secp256k1 ctime tests when tests are not being built (Hennadii Stepanov)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/pull/30791#issuecomment-2340860619:
> Building with a fuzz engine fails, because the ctime tests are auto-detected in cmake, based on whether or not valgrind-devel is installed or not.
ACKs for top commit:
maflcko:
re-review ACK 23eedc5d1e
fanquake:
ACK 23eedc5d1e
Tree-SHA512: bfc0f2798acd36be9c52073d578b42c002606c60ef3fe8ef633eaea4f5382a3e9765d31637e4c25d8b71fd70473b29c24af4732e55e5183f27b48725b61fa15b
2ad560139b Remove unused src_dir param from run_tests (Lőrinc)
Pull request description:
The `src_dir` usage was removed in a8a2e364ac (diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598), making the parameter unused.
Top commit has no ACKs.
Tree-SHA512: 1fd8b93811b4ab467ba5a160a4fe204e9606e1bf237c7595ed6f8b7821cf59d2a776c0e1e154852a45b2a35e5bdbd8996314e4f63a9c750f21b9a17875cb636a
f15e817811 build: add more CMake presets (dev-mode, libfuzzer, libfuzzer-nosan) (Pieter Wuille)
Pull request description:
Add three more cmake presets to the project-wide `CMakePresets.json` file:
* `dev-mode`: enables all features and dependencies
* `libfuzzer`: builds for fuzzing with libfuzzer and the typical sanitizers (but not the optional ones that require suppressions) enabled.
* `libfuzzer-nosan`: builds for fuzzing with libfuzzer and no (other) sanitizers
... and then uses these in some documentation.
ACKs for top commit:
ryanofsky:
Code review ACK f15e817811. This change is much needed to simplify my command line.
TheCharlatan:
ACK f15e817811
Tree-SHA512: a5f67bb7119fd36832ca5eb7189db04bfaf88f954aa461bfb2aeb866469057b0d0272835c418bc3a264c30dd8fba6d2e2cc8a6741a889f28f52c1c09b3ba9704
be4f78275f contrib: test for FORTIFY_SOURCE in security-check.py (fanquake)
Pull request description:
Test for the existence of fortified functions in the ELF release binaries.
Currently skips `bitcoin-util` and checks for RISC-V.
ACKs for top commit:
TheCharlatan:
ACK be4f78275f
Tree-SHA512: decea5f359f1e673aa0119916f674f409a13b69db7da366cd95c1540201e117ff5a979da67bc2517fe786c2ac23d1006a9aaf662d7eadeec35da6aae4998c065
7b04fabe2d build: Introduce "Kernel" installation component (Hennadii Stepanov)
Pull request description:
This PR enables building and installing only `libbitcoinkernel`, without the need to disable other targets during the project build system generation:
```
$ rm -rf build && cmake -B build -DBUILD_KERNEL_LIB=ON
$ cmake --build build --target bitcoinkernel
$ cmake --install build --component Kernel --prefix /home/hebasto/INSTALL
-- Install configuration: "RelWithDebInfo"
-- Installing: /home/hebasto/INSTALL/lib/libbitcoinkernel.so
```
Please note, that only the `bitcoinkernel` target is being built.
Related to https://github.com/bitcoin/bitcoin/issues/30801 and https://github.com/bitcoin/bitcoin/pull/30814.
ACKs for top commit:
TheCharlatan:
ACK 7b04fabe2d
ryanofsky:
Code review ACK 7b04fabe2d
Tree-SHA512: eac114dde059e47c91938a4a9108fc0fc693b5342ed3b6ecb971615be8ad3225b9985aae12d6ad18e673edf1bd39a5ecf259c1b61734f221669091bf2ce93a67
5ba03e7d35 build: Use CMake's default permissions in macOS `deploy` target (Hennadii Stepanov)
Pull request description:
This PR ensures that the file permissions in macOS `zip` archives are independent of the user's `umask` value.
Fixes https://github.com/bitcoin/bitcoin/issues/30815.
ACKs for top commit:
fanquake:
ACK 5ba03e7d35 - I'm going to merge this now so we return to usable (comparable) guix builds.
Tree-SHA512: 78f724cd3ffd5c1fd5fc1b4832f1e8154c62723f3de5ac9599f44715cbd08a3dfbb806801411c55069773d2e34c9f8cab25585dbad2f032c36b68dd83cb51847
c45186ca54 ci: Switch from `make` to `cmake --build` (Hennadii Stepanov)
6e5f33af58 ci: Handle log files regardless of CMake's version (Hennadii Stepanov)
Pull request description:
This PR addresses the change in logging that [happened](https://cmake.org/cmake/help/latest/release/3.26.html#configure-log) in CMake 3.26.
Additionally, the `make` invocation replaced with `cmake --build`.
Here are examples of the CI logs:
- for a an error during the build system generation: https://cirrus-ci.com/build/5210987156996096
- for a compiler error: https://cirrus-ci.com/build/4617660913156096
ACKs for top commit:
maflcko:
review ACK c45186ca54
fanquake:
ACK c45186ca54
Tree-SHA512: 2096f08c482ab9e10056cd4ec694ce40996243e2a1af2212dfff8cccbf0f51391d9a3dc396f7bba4f2877072a13a42bf667a02a44eab44e917aafb14d04e8e39
b07fe666f2 build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake` (Hennadii Stepanov)
Pull request description:
This PR aims to reduce build time by replacing multiple `file(WRITE|APPEND ...)` commands with a single `file(WRITE ...)` command.
Due to differences in implementation (e.g., filesystem design, system calls, caching), a noticeable improvement in build time is observed only on Windows.
Additionally, the code has been refactored to remove the `remainder` local variables.
ACKs for top commit:
sipsorcery:
tACK b07fe666f2
maflcko:
review ACK b07fe666f2
TheCharlatan:
ACK b07fe666f2
Tree-SHA512: 6ed3ae8fe7d8859af38d83918eddf7cb318607787863b95589f4a7a45a36f8c4bd1c01e366078d0515115c121bc857dc63471e52ff26fc49edbc8bb69875e947
