highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-16 17:48:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35528: test: doc: remove --perf profiling from functional test framework

Browse Source 
9fae7e9886 test: doc: remove `--perf` profiling from functional test framework (Sebastian Falbesoner)

Pull request description:

  This PR is an alternative to #35509. Rather than fixing, it removes the `--perf` option / `profile_with_perf` context manager features (both introduced in #14519) from the functional test framework. Given that no developer apparently even noticed that it's been broken for more than a year, we can conclude that it's largely unused and getting rid of it to reduce maintenance burden seems a reasonable choice. So far, neither in #35509 nor [on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2026-06-12#1227556;) anyone has signaled strong interest to use or wanting to keep this feature.

  The corresponding test documentation is removed as well, though the mentioning of `perf` profiling is still kept in the general developer docs -- only the reference to functional test framework integration is removed there.

ACKs for top commit:
  l0rinc:
    lightweight code review ACK 9fae7e9886
  maflcko:
    lgtm ACK 9fae7e9886  While this seems useful, no one using it is a good reason to remove it. If a user comes after this is merged, it should be trivial to revert.

Tree-SHA512: cf7bdce72aed877c7dfa52a230840e0729e74b00f0c40c6ceecf04957707f4699f123c2cde5fc1c4ee21f4b1b14319c74acc78cd8066eab2b9a4d6efb4d11539
This commit is contained in:
merge-script
2026-06-15 15:04:45 +02:00
parent 46927cf82c 9fae7e9886
commit 355fffb8cc
6 changed files with 3 additions and 163 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
doc/developer-notes.md
View File

@@ -562,9 +562,8 @@ For IWYU pragmas, prefer adding a nearby source comment that explains why the an
Profiling is a good way to get a precise idea of where time is being spent in
code. One tool for doing profiling on Linux platforms is called
[`perf`](https://www.brendangregg.com/perf.html), and has been integrated into
the functional test framework. Perf can observe a running process and sample
(at some frequency) where its execution is.
[`perf`](https://www.brendangregg.com/perf.html). It can observe a running
process and sample (at some frequency) where its execution is.
Perf installation is contingent on which kernel version you're running; see
[this thread](https://askubuntu.com/questions/50145/how-to-install-perf-monitoring-tool)
@@ -599,8 +598,6 @@ perf report --stdio | c++filt | less
or using a graphical tool like [Hotspot](https://github.com/KDAB/hotspot).
See the functional test documentation for how to invoke perf within tests.
### Valgrind
Valgrind is a programming tool for memory debugging, memory leak detection, and

20
test/README.md
View File

@@ -318,26 +318,6 @@ Often while debugging RPC calls in functional tests, the test might time out bef
process can return a response. Use `--timeout-factor 0` to disable all RPC timeouts for that particular
functional test. Ex: `build/test/functional/wallet_hd.py --timeout-factor 0`.
##### Profiling
An easy way to profile node performance during functional tests is provided
for Linux platforms using `perf`.
Perf will sample the running node and will generate profile data in the node's
datadir. The profile data can then be presented using `perf report` or a graphical
tool like [hotspot](https://github.com/KDAB/hotspot).
To generate a profile during test suite runs, use the `--perf` flag.
To see render the output to text, run
```sh
perf report -i /path/to/datadir/send-big-msgs.perf.data.xxxx --stdio | c++filt | less
```
For ways to generate more granular profiles, see the README in
[test/functional](/test/functional).
### Lint tests
See the README in [test/lint](/test/lint).

33
test/functional/README.md
View File

@@ -164,36 +164,3 @@ Test-only secp256k1 elliptic curve implementation
#### [blocktools.py](test_framework/blocktools.py)
Helper functions for creating blocks and transactions.
### Benchmarking with perf
An easy way to profile node performance during functional tests is provided
for Linux platforms using `perf`.
Perf will sample the running node and will generate profile data in the node's
datadir. The profile data can then be presented using `perf report` or a graphical
tool like [hotspot](https://github.com/KDAB/hotspot).
There are two ways of invoking perf: one is to use the `--perf` flag when
running tests, which will profile each node during the entire test run: perf
begins to profile when the node starts and ends when it shuts down. The other
way is the use the `profile_with_perf` context manager, e.g.
```python
with node.profile_with_perf("send-big-msgs"):
# Perform activity on the node you're interested in profiling, e.g.:
for _ in range(10000):
node.p2ps[0].send_without_ping(some_large_message)
```
To see useful textual output, run
```sh
perf report -i /path/to/datadir/send-big-msgs.perf.data.xxxx --stdio | c++filt | less
```
#### See also:
- [Installing perf](https://askubuntu.com/q/50145)
- [Perf examples](https://www.brendangregg.com/perf.html)
- [Hotspot](https://github.com/KDAB/hotspot): a GUI for perf output analysis

1
test/functional/test-shell.md
View File

@@ -180,7 +180,6 @@ can be called after the TestShell is shut down.
| `loglevel` | `INFO` | Logs events at this level and higher. Can be set to `DEBUG`, `INFO`, `WARNING`, `ERROR` or `CRITICAL`. |
| `nocleanup` | `False` | Cleans up temporary test directory if set to `True` during `shutdown`. |
| `num_nodes` | `1` | Sets the number of initialized bitcoind processes. |
| `perf` | False | Profiles running nodes with `perf` for the duration of the test if set to `True`. |
| `rpc_timeout` | `60` | Sets the RPC server timeout for the underlying bitcoind processes. |
| `setup_clean_chain` | `False` | A 200-block-long chain is initialized from cache by default. Instead, `setup_clean_chain` initializes an empty blockchain if set to `True`. |
| `randomseed` | Random Integer | `TestShell().options.randomseed` is a member of `TestShell` which can be accessed during a test to seed a random generator. User can override default with a constant value for reproducible test runs. |

9
test/functional/test_framework/test_framework.py
View File

@@ -191,8 +191,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
help="Attach a python debugger if test fails")
parser.add_argument("--usecli", dest="usecli", default=False, action="store_true",
help="use bitcoin-cli instead of RPC for all commands")
parser.add_argument("--perf", dest="perf", default=False, action="store_true",
help="profile running nodes with perf for the duration of the test")
parser.add_argument("--valgrind", dest="valgrind", default=False, action="store_true",
help="Run binaries under the valgrind memory error detector: Expect at least a ~10x slowdown. Does not apply to previous release binaries.")
parser.add_argument("--randomseed", type=int,
@@ -292,15 +290,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
should_clean_up = (
not self.options.nocleanup and
self.success != TestStatus.FAILED and
not self.options.perf
self.success != TestStatus.FAILED
)
if should_clean_up:
self.log.info("Cleaning up {} on exit".format(self.options.tmpdir))
cleanup_tree_on_exit = True
elif self.options.perf:
self.log.warning("Not cleaning up dir {} due to perf data".format(self.options.tmpdir))
cleanup_tree_on_exit = False
else:
self.log.warning("Not cleaning up dir {}".format(self.options.tmpdir))
cleanup_tree_on_exit = False
@@ -482,7 +476,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
extra_conf=extra_confs[i],
extra_args=args,
use_cli=self.options.usecli,
start_perf=self.options.perf,
v2transport=self.options.v2transport,
uses_wallet=self.uses_wallet,
)

96
test/functional/test_framework/test_node.py
View File

@@ -18,7 +18,6 @@ import tempfile
import time
import urllib.parse
import collections
import shlex
import sys
from collections.abc import Iterable
from pathlib import Path
@@ -109,18 +108,11 @@ class TestNode():
extra_conf=None,
extra_args=None,
use_cli=False,
start_perf=False,
version=None,
v2transport=False,
uses_wallet=False,
ipcbind=False,
):
"""
Kwargs:
start_perf (bool): If True, begin profiling the node with `perf` as soon as
the node starts.
"""
self.index = i
self.datadir_path = datadir_path
self.bitcoinconf = self.datadir_path / "bitcoin.conf"
@@ -191,7 +183,6 @@ class TestNode():
self.cli = None
self.use_cli = use_cli
self.start_perf = start_perf
self.running = False
self.process = None
@@ -200,8 +191,6 @@ class TestNode():
self.reuse_http_connections = True # Must be set before create_new_rpc_connection(), i.e. before restarting node
self.url = None
self.log = logging.getLogger('TestFramework.node%d' % i)
# Cache perf subprocesses here by their data output filename.
self.perf_subprocesses = {}
self.p2ps = []
@@ -298,9 +287,6 @@ class TestNode():
self.running = True
self.log.debug("bitcoind started, waiting for RPC to come up")
if self.start_perf:
self._start_perf()
def create_new_rpc_connection(self, *, mode="AUTO", client_timeout=None):
"""Create an additional RPC connection, likely to be used in a new thread."""
mode = RPCConnectionType[mode]
@@ -490,10 +476,6 @@ class TestNode():
else:
self.stop()
# If there are any running perf processes, stop them.
for profile_name in tuple(self.perf_subprocesses.keys()):
self._stop_perf(profile_name)
del self.p2ps[:]
assert (not expected_stderr) or wait_until_stopped # Must wait to check stderr
@@ -679,84 +661,6 @@ class TestNode():
yield
self.wait_until(lambda: get_highest_peer_id() > initial_peer_id, timeout=timeout)
@contextlib.contextmanager
def profile_with_perf(self, profile_name: str):
"""
Context manager that allows easy profiling of node activity using `perf`.
See `test/functional/README.md` for details on perf usage.
Args:
profile_name: This string will be appended to the
profile data filename generated by perf.
"""
subp = self._start_perf(profile_name)
yield
if subp:
self._stop_perf(profile_name)
def _start_perf(self, profile_name=None):
"""Start a perf process to profile this node.
Returns the subprocess running perf."""
subp = None
def test_success(cmd):
return subprocess.call(
# shell=True required for pipe use below
cmd, shell=True,
stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL) == 0
if platform.system() != 'Linux':
self.log.warning("Can't profile with perf; only available on Linux platforms")
return None
if not test_success('which perf'):
self.log.warning("Can't profile with perf; must install perf-tools")
return None
if not test_success('readelf -S {} | grep .debug_str'.format(shlex.quote(self.binary))):
self.log.warning(
"perf output won't be very useful without debug symbols compiled into bitcoind")
output_path = tempfile.NamedTemporaryFile(
dir=self.datadir_path,
prefix="{}.perf.data.".format(profile_name or 'test'),
delete=False,
).name
cmd = [
'perf', 'record',
'-g', # Record the callgraph.
'--call-graph', 'dwarf', # Compatibility for gcc's --fomit-frame-pointer.
'-F', '101', # Sampling frequency in Hz.
'-p', str(self.process.pid),
'-o', output_path,
]
subp = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
self.perf_subprocesses[profile_name] = subp
return subp
def _stop_perf(self, profile_name):
"""Stop (and pop) a perf subprocess."""
subp = self.perf_subprocesses.pop(profile_name)
output_path = subp.args[subp.args.index('-o') + 1]
subp.terminate()
subp.wait(timeout=10)
stderr = subp.stderr.read().decode()
if 'Consider tweaking /proc/sys/kernel/perf_event_paranoid' in stderr:
self.log.warning(
"perf couldn't collect data! Try "
"'sudo sysctl -w kernel.perf_event_paranoid=-1'")
else:
report_cmd = "perf report -i {}".format(output_path)
self.log.info("See perf output by running '{}'".format(report_cmd))
def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, match=ErrorMatch.FULL_TEXT, *args, **kwargs):
"""Attempt to start the node and expect it to raise an error.