diff --git a/doc/developer-notes.md b/doc/developer-notes.md index e44a4b91093..af242291c6a 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -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 diff --git a/test/README.md b/test/README.md index 162815d6488..0b34f52c1e6 100644 --- a/test/README.md +++ b/test/README.md @@ -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). diff --git a/test/functional/README.md b/test/functional/README.md index cf0f2fd0e37..2de034500f8 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -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 diff --git a/test/functional/test-shell.md b/test/functional/test-shell.md index 526c67f6905..0c2b7d3d43a 100644 --- a/test/functional/test-shell.md +++ b/test/functional/test-shell.md @@ -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. | diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 1e2ac633966..8f5fb2397ba 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -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, ) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 22d76cacbcf..77a7fcb4c8b 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -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.