13782b8ba8 docs: add perf section to developer docs (James O'Beirne)
58180b5fd4 tests: add utility to easily profile node performance with perf (James O'Beirne)
Pull request description:
Adds a context manager to easily (and selectively) profile node performance during functional test execution using `perf`.
While writing some tests, I encountered some odd bitcoind slowness. I wrote up a utility (`TestNode.profile_with_perf`) that generates performance diagnostics for a node by running `perf` during the execution of a particular region of test code.
`perf` usage is detailed in the excellent (and sadly unmerged) https://github.com/bitcoin/bitcoin/pull/12649; all due props to @eklitzke.
### Example
```python
with node.profile_with_perf("large-msgs"):
for i in range(200):
node.p2p.send_message(some_large_msg)
node.p2p.sync_with_ping()
```
This generates a perf data file in the test node's datadir (`/tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data`).
Running `perf report` generates nice output about where the node spent most of its time while running that part of the test:
```bash
$ perf report -i /tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data --stdio \
| c++filt \
| less
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 135 of event 'cycles:pp'
# Event count (approx.): 1458205679493582
#
# Children Self Command Shared Object Symbol
# ........ ........ ............... ................... ........................................................................................................................................................................................................................................................................
#
70.14% 0.00% bitcoin-net bitcoind [.] CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
|
---CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
70.14% 0.00% bitcoin-net bitcoind [.] CNetMessage::readData(char const*, unsigned int)
|
---CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
35.52% 0.00% bitcoin-net bitcoind [.] std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
|
---std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
...
```
Tree-SHA512: 9ac4ceaa88818d5eca00994e8e3c8ad42ae019550d6583972a0a4f7b0c4f61032e3d0c476b4ae58756bc5eb8f8015a19a7fc26c095bd588f31d49a37ed0c6b3e
688f665a5e526fda0fb797bf617412fe9cbe64fd Scripts and tools & Docs: Used #!/usr/bin/env bash instead of obsolete #!/bin/bash, added linting for .sh files shebang and updated the Developer Notes. (vim88)
Pull request description:
As it was discussed in [#13510](https://github.com/bitcoin/bitcoin/pull/13510), it is better to use `#!/usr/bin/env bash` instead of `#!/bin/bash`.
Tree-SHA512: 25f71eb9a6a0cdc91568b5c6863205c5fe095f77a69e633503a2ac7805bd9013af8538e538c0c666ce96a28e3f43ce7a8df5f08d4ff007723bb588d85674f2da
6c6ee8af8 Less confusing documentation for `torpassword` (Chakib Benziane)
Pull request description:
Rebased & squashed #14609.
> The current documentation leads the reader to think hash-password is an other option.
This change is less confusing and make it clear how to use this option.
Tree-SHA512: b94223715f1d28a8218e501117faa103d7cc791333e6d9a40e9253756070316d41de73e3b4593cd1fa9e1823fa8e8de4938cf0de89117a6e4649401d877e3477
f24ed6d39f963e7f1c0b4dbd9b784132ef975f2a Delete README_osx.md and move its contents into build-osx.md (Martin Erlandsson)
Pull request description:
With its current name, the file `doc/README_osx.md` looks like an entry point README for OSX users, but it only contains specific instructions on how to build a DMG.
This PR deletes the file and moves the contents of the file into `doc/build-osx.md`.
Tree-SHA512: 2636b9da967f2a4c0d68cb9a157fb3db137bdb8fbff5d9d004f28b5d816e9c27fddc5c403e6b0c363d1dc9ddc7cac8b295efa01fc691126c0e36e21bb9b3cbd3
75a4bf699fa6bdefa1b3d8cd405ea822d6ee01c0 Update release-process.md to include RC version bumping (Andrew Chow)
04b0bc7425e43de90856beeb1f33653db109fecd build: include rc number in version number (Andrew Chow)
895e6bbb2241e9175463734f3677398a9f38f0f8 build: if VERSION_BUILD is non-zero, include it in the package version (Andrew Chow)
Pull request description:
As noted on IRC, the filenames of the gitian build results do not contain the 4th digit of the version number if it has one, e.g. 0.17.0.1 produces files with the number 0.17.0. Furthermore, when RC's are built, the resulting filenames are of the release version and do not include `rc` in them. This occurs because `configure.ac` is written to create version numbers of the form `major.minor.rev` instead of `major.minor.rev.build` and without any rc version as it does not handle rc numbers.
This PR changes `configure.ac` to include the build number if it is greater than 0. It will also include the rc number if it is greater than 0. So the filenames of the gitian builds will now contain the full version number.
This behavior can be tested by setting `_CLIENT_VERSION_BUILD` and `_CLIENT_VERSION_RC` to non-zero values and then doing `make dist`. A tar file should be created with the correct versioning.
Tree-SHA512: b77990485f2c7770be897dc136737cd805306afff9882ebef7170741f363203587356ccf8bec83163268ace1bd77433fbd2ba8c213f993677bfb867d99a0bbe7
fa4da3c058 [doc] conf: Remove deprecated options from docs, Other cleanup (MarcoFalke)
Pull request description:
Some dumb fixes, like removing the mention that free transactions are still a thing or that rpcuser/pass should be used (as opposed to rpcauth or rpc cookie).
Combined with other fixes because I don't want to create 3 pull requests:
* conf: Remove deprecated options from docs
* Remove only mention of MIT/X11
* Link to developer notes in README.md
Tree-SHA512: 9e45dc6c63037e7618cf3c871d7d9e65b66f1a952f91a6e623d97d90171e29bc40299a06029c4dc21a0f579e68021e3663186bd3a65e3ab333aff711f7dcb2bf
fa8ced32a60dea37ac169241cf9a1f708ef46c4b doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de79faaca2b088fcbe7f76701faa9bff236 test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e7b4540857261aed948b94b6b2bfdbc3d1 test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7329d3e74d46ab98b93772b1832a3f1819 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)
Pull request description:
This is de-facto no longer hidden
ACKs for commit fa8ced:
jamesob:
utACK fa8ced32a6
Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
19efc01aec6b0d8750413fa1b721e04aaecf8f73 Add PSBT documentation (Pieter Wuille)
Pull request description:
This is just some initial text to get going; other contributions welcome.
I'd like to include other workflows, such as hardware wallets and (manual) coinjoins. However, the former will in practice require PSBT interfaces for existing hardware devices, and the second can really use some extra RPCs first.
Tree-SHA512: 951e475e31bb2ea9ab5d84d139b8bc436153ad035185f00ad1d56afc0c6f7c4de8176a785a6d0c38bb3fd9cbf318e513e1a032e83e1da99ded5d43a36f9cbc60
6780a095d8526a46c3c2b20a14831687e9fc2462 doc: remove obsolete `okSafeMode` RPC guideline from developer notes (Sebastian Falbesoner)
Pull request description:
Since the flag has been removed from the RPC command table in commit ec6902d0ea (PR #11179), this guideline is not relevant anymore and can be removed.
ACKs for top commit:
MarcoFalke:
ACK 6780a095d8526a46c3c2b20a14831687e9fc2462. Also, safe mode was removed completely in commit 2ae705d841
Tree-SHA512: 2a6b002ae302e979ce403171b79a892e32f5083792c3b0b8204edb5eb08c6b24ab77bbeeae0e3bb6d6564a6f1678cfce00eb7b5b82063b7741f89a96b0c0aef3
aa929abf8dc022e900755234c857541faeea8239 [docs] Update developer notes to discourage very long lines (John Newbery)
Pull request description:
Mandatory rules on line lengths are bad - there will always be cases where a longer line is more readable than the alternative.
However, very long lines for no good reason _do_ hurt readability. For example, this declaration in validation.h is 274 chars:
```c++
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
```
That won't fit on one line without wrapping on my 27" monitor with a comfortable font size. Much easier to read is something like:
```c++
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams,
CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock,
ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool)
EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
```
Therefore, _discourage_ (don't forbid) line lengths greater than 100 characters in our developer style guide.
100 chars is somewhat arbitrary. The old standard was 80, but that seems very limiting with modern displays.
ACKs for top commit:
fanquake:
ACK aa929abf8dc022e900755234c857541faeea8239 - this is basically just something to point too when a PR has unreasonably long lines for no particularly reason.
practicalswift:
ACK aa929abf8dc022e900755234c857541faeea8239
amitiuttarwar:
ACK aa929abf8dc022e900755234c857541faeea8239
theStack:
ACK aa929abf8dc022e900755234c857541faeea8239
glozow:
ACK aa929abf8d
Tree-SHA512: 17f1b11f811137497ede8851ede93fa612dc622922b5ad7ac8f065ea026d9a718db5b92325754b74d24012b4d45c4e2cd5cd439a6a8d34bbabf5da927d783970
5c3eaf9983043db1b61a98c95d692a6958670b86 doc: Add warnings for http interfaces limitations (Fabian Jahr)
Pull request description:
`libevent`, which is used for our rest interface, can use up all of the available file descriptors in a system if too many connections are opened at once. If a new block is connected at the same time and can not be written to disk because there are no file descriptors available, the node crashes. Based on my investigation so far the issue is best solved upstream which means we have to wait for the next release (2.2). In the meantime it would be good if we would warn users of this limitation.
See #11368 for more background.
ACKs for top commit:
MarcoFalke:
ACK 5c3eaf9983043db1b61a98c95d692a6958670b86
Tree-SHA512: 73914538588477ead19068f5832fdcc8e0eb736e51f73b3aca501c93165e5ad634c2511a3fcffff251adcd3efda23a742b48211ad9d3b2a29cdeac17201d06a1
* Update to leveldb upstream using subtree merge
* Import crc32c using subtree merge as as 'src/crc32c'
* build: Update build system for new leveldb
Upstream leveldb switched build systems, which means we need to define
a few different values.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* doc: Add crc32c subtree to developer notes
* test: Add crc32c to subtree check linter
* test: Add crc32c exception to various linters and generation scripts
* build: Add LCOV exception for crc32c
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* build: CRC32C build system integration
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
478c11dde326e2ff0480c14f76f9f6b52a7bdfd0 Correct scripted-diff example link (Yahia Chiheb)
Pull request description:
ACKs for top commit:
fanquake:
ACK 478c11dde326e2ff0480c14f76f9f6b52a7bdfd0
Tree-SHA512: 3bc741a79db9bd7abb17ef11f697b768565ec01303a5823ee6a7d8dfa6e888a99a15e9eda69f97a912abc3fd56a54f698f9a580596511bc9bcf62a6870b273f6
4928a995e9799c6c7ea84fa1efc4fef5b2ff7683 [doc] fix git add argument (Michael Polzer)
Pull request description:
[`A`](https://git-scm.com/docs/git-add#Documentation/git-add.txt--A) is the correct flag.
ACKs for top commit:
fanquake:
ACK 4928a995e9799c6c7ea84fa1efc4fef5b2ff7683 - checked that [`A`](https://git-scm.com/docs/git-add#Documentation/git-add.txt--A) and not `a` is the correct flag.
Tree-SHA512: 7e656ca9688b04ad2ef577aa1847799a34a377f5e6dfe4fd052a95d3dd98798dc10957e7f54164900ac1271f05e788ec4861026f53b910e369b0845532387cf4
6094222de7820d235e6e8c66e589aa71db08c077 use preferred shebang approach for documentation (hackerrdave)
Pull request description:
Documentation update to use recommended shebang approach mentioned in the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#shebang)
ACKs for top commit:
hebasto:
ACK 6094222de7820d235e6e8c66e589aa71db08c077, I have reviewed the code, and it looks OK, I agree it can be merged.
Tree-SHA512: fc58632f0a6fa82c7abdddfac4897f082110d647426d2b468cba6fabf6b34a015fcad47e5b26be98e629b8b0417b8781e8d89da67189e20da228b97b17f1a532
d97fac422eaaefe13e1ed376e617882a100872ae Add a link from ZMQ doc to ZMQ example in contrib/ (Damian Mee)
Pull request description:
No code changes :). Only a small convenience improvement in zmq doc.
ACKs for top commit:
fanquake:
ACK d97fac422eaaefe13e1ed376e617882a100872ae
Tree-SHA512: f05a8a7a77c0a698637fd24ffc94d0d617743b434f46695a56576a53331ede254aeece416baf3f8275ae4dfad85ae6e14d1920aa32af53150847420a176d90fb
ff6549c3c84ca7324032dbc37744645bf2fe1c3e fix: update rest info on block size and json (Chris Abrams)
Pull request description:
Addressing the ambiguous block size text in rest docs: https://github.com/bitcoin/bitcoin/issues/18703
Also makes sure to let developers know there is `.json` option for the rest output format.
ACKs for top commit:
MarcoFalke:
ACK ff6549c3c84ca7324032dbc37744645bf2fe1c3e
promag:
ACK ff6549c3c84ca7324032dbc37744645bf2fe1c3e.
Tree-SHA512: 9ef93c1432d650b1f9599778ba092c1ca5b084a537af257078e1c713c76c5d3a4cc4b1ede8a2489964be8ed0303ad8bea58c1cb4759bbb9b24dbdebfec8001d3
adbe15504713ddba6e9c024c59d977675d49e350 doc: Add some better examples for scripted diff (Wladimir J. van der Laan)
Pull request description:
The current example isn't too great, for example it uses `find` instead of `git ls-files`. Add a subsection with suggestions and examples.
Feel free to propose some other great examples to add.
ACKs for top commit:
hebasto:
re-ACK adbe15504713ddba6e9c024c59d977675d49e350
Tree-SHA512: 38f03716a122a1791c93abc052ea7572a3d2108b3d0d93dc95d3c4a7eb190c6b639d1cc66e4f74d378c4b11d6951dbd901d0973792f8f13cbeb9d9dcf4f8e037
14c6a2de1a4cf8cc17116d418242709ba2519b9e [doc] update brew instruction for doxygen (Sjors Provoost)
Pull request description:
I noticed while testing #16912 that `brew install doxygen --with-graphviz` no long works. Instead you need to use `brew install graphviz doxygen`.
ACKs for top commit:
fanquake:
ACK 14c6a2de1a4cf8cc17116d418242709ba2519b9e - tested a `make docs` on macOS with and without `graphviz` (`dot`) available.
Tree-SHA512: 2682568e558c16e9e0a657421c449b74cc14a89771844c1c88623fb75b07b89afb63c45a919eb7b9c3dba9bdfaef21489b5f7ea45a08d8d5da18614657c19e47
d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad Add window final block height to getchaintxstats (Jonathan "Duke" Leto)
Pull request description:
This patch is motivated by the desire to make the output of `getchaintxstats` more useful and optimized for applications to consume and render the data.
Firstly, this data is already available to the RPC, no additional work is done. Currently additional RPC calls will be needed to look up the height of the final block in the window or the block height that began the window.
By adding the block height of the final block in the window, the JSON is "self-contained" and applications can calculate the exact block height range of the window with no additional RPC requests.
For example, a web application which wants to render historical information for `getchaintxstats` RPC on various window sizes might call the RPC with various window lengths, once per day, and store the JSON results somewhere. Because the final block height of each dataset is included, it's no extra work to determine the exact block window range of each JSON response.
ACKs for top commit:
promag:
ACK d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad.
Tree-SHA512: fd4952c125f81a4ad18f7c78498c6b3e265b93cb574832166ac25596321ce84957f971f3f78f37d7e42638dc65f2a5d4d760f289873c9c2f2a82eb00a0f87c3f
1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0 doc: add default bitcoin.conf locations (Chuf)
Pull request description:
Added default bitcoin.conf data directories and paths
ACKs for top commit:
practicalswift:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0
ryanofsky:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0
fanquake:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0 - Already three ACKs and lots of discussion here, so I'm going to merge, and the other comment
Tree-SHA512: 8bb1ed9868c5d171b6791bd6dc9598eddfdf64977d327ff4f333323cef8e3e76b1a67da21e4199f008a12f5610ac6dc6f34f4a13235e8846754eb6d6e5075da4
9452802480bd154e23771230bbdfebde1dbaa941 doc: Tidy up shadowing section (João Barbosa)
Pull request description:
Removes the example because it violates the code format.
ACKs for top commit:
MarcoFalke:
unsigned ACK 9452802480bd154e23771230bbdfebde1dbaa941
ryanofsky:
ACK 9452802480bd154e23771230bbdfebde1dbaa941
fanquake:
ACK 9452802480bd154e23771230bbdfebde1dbaa941 - Thanks for following up.
Tree-SHA512: 1fc31355d368225713298da7803e39e99014fbfcd229f2d3b56c082de95ab2965e51c80b172a5abce4646c53f845fa62a6d94d5df714e7835cac07a8ec7d5da7
64b27c46e4c21fc7902a69d8ddb6791ef417c4af docs: add reduce-memory.md (fanquake)
Pull request description:
Following some discussion in https://github.com/bitcoin-core/docs/issues/50, this adds Wladimir's [reducing bitcoind memory usage gist](https://gist.github.com/laanwj/efe29c7661ce9b6620a7) to `/doc`.
The conclusion seemed to be that if the main repo already has [reduce-traffic.md](https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-traffic.md), then we could also add `reduce-memory.md`.
ACKs for top commit:
practicalswift:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af
hebasto:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af, I have reviewed the changes and they look OK, I agree they can be merged. Also a link from `/doc/README.md` has been tested.
jonasschnelli:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af
Tree-SHA512: 0ab3035403e5145cfe33c29990a8d082df834ac6602b4ad6bfa821523d57e8451f0cde3017fbf3c2c4e0b34941b6374909d11d27f9598e211bbc14accd487be1
5a88ea7c67448748a63ac7963c70a047a5daca79 doc: remove orphaned header in developer notes (Jon Atack)
Pull request description:
The "Git and GitHub tips" section was moved from doc/developer-notes.md to doc/productivity.md in 5b76c31, but the header link to that long-gone section in the developer notes remains and needs to go.
So long, Git and GitHub tips, we barely knew ya.
ACKs for commit 5a88ea:
fanquake:
ACK 5a88ea7c67448748a63ac7963c70a047a5daca79
Tree-SHA512: d2a0bea27cd9209adec2127fae5e336c44771aa46af7c544fa2c80a3df4868adafbccc30ef370369404b882fa009f7198cbf41777afd30eccf0c21a7eb1d0ad1
fa2f991fa23fcc2c96d48fc3dd7faa41c959e8ae doc: add note on precedence of options in bitcoin.conf (Torkel Rogstad)
Pull request description:
This was a good addition to the docs, but the PR was closed. So I've cherry-picked the commit and fixed up Russ's comments as well as the linter issue.
ACKs for top commit:
laanwj:
ACK fa2f991fa23fcc2c96d48fc3dd7faa41c959e8ae
hebasto:
ACK fa2f991fa23fcc2c96d48fc3dd7faa41c959e8ae, I have reviewed the code and it looks OK, I agree it can be merged.
jamesob:
ACK fa2f991fa2
jonatack:
ACK fa2f991fa23fcc2c96d48fc3dd7faa41c959e8ae
ryanofsky:
ACK fa2f991fa23fcc2c96d48fc3dd7faa41c959e8ae. Only suggested changes since previous review.
Tree-SHA512: d8e7bac19e85ad32205652c3c3036766c611cae52e6e3e8af66a2da054659d914dc153d0cf4ace9c0fa7b41f2a8d74d0edd8d83fe7e984b93d70c01a388cf8ec
01971da9bd docs: Add productivity notes for "dummy rebases" (Carl Dong)
Pull request description:
When rebasing, we often want to do a "dummy rebase" whereby we are not rebasing over an updated master. This is because rebases can be confusing enough already, and we don't want to resolve upstream conflicts together with our local rebase conflicts due to fixup commits, commit rearrangements, and such. This productivity section details how to do such "dummy rebase"s.
ACKs for commit 01971d:
Tree-SHA512: 241a451cec01dc9a01a2286bdee1441cac6d28007f5b173345744d2abf436da916c3f2553ff0d1c5b3687055107b37872dda9529288645e4bae7b3cb46923b7e
201393f932 Align code example with clang-format (Hennadii Stepanov)
Pull request description:
With this PR running [clang-format-diff.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/clang-format-diff.py) on the code example will not fire a format adjustment.
ACKs for commit 201393:
MarcoFalke:
trivial ACK 201393f93268f6775d1b5d54119a7210628b333d
Tree-SHA512: 825c5e8cfba1bc140c2dfc38b82c5eec268b82b528af4301f25dfacc1f4f0788e268e72e8512f7ce001be665a8b07964a0af832fd9a1c6bd1c27d252f93619bc
5801dd628d docs: Add more tips to productivity.md (gwillen)
Pull request description:
Add advice to productivity.md on:
- Using ccache to optimal effect
- The with-incompatible-bdb configure option
- Building less than the entire set of targets
ACKs for commit 5801dd:
promag:
utACK 5801dd6.
MarcoFalke:
utACK 5801dd6
Tree-SHA512: 2138acd4bf5a27ecaa9a02fb2141903d01ee199ba85ccf6a5ad6a0a4dabf4447d043108cdd86998801b0282e899f70892f9337b0b6dc59c6d1f0fccf61adb4e4
