From 53bec6882740c0a61053515ac6ecafc5e139babb Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Tue, 30 May 2023 15:57:00 +0200 Subject: [PATCH 01/11] Support grouping profilers under common runtime --- gprofiler/main.py | 38 +++--- gprofiler/profilers/factory.py | 75 ++++++----- gprofiler/profilers/profiler_base.py | 12 ++ gprofiler/profilers/python.py | 3 + gprofiler/profilers/registry.py | 128 ++++++++++++------ tests/test_python.py | 45 ++++++- tests/test_runtime_profilers.py | 185 +++++++++++++++++++++++++++ 7 files changed, 400 insertions(+), 86 deletions(-) create mode 100644 tests/test_runtime_profilers.py diff --git a/gprofiler/main.py b/gprofiler/main.py index a5680b649..9f5077f28 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -49,7 +49,12 @@ from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.profiler_base import NoopProfiler, ProcessProfilerBase, ProfilerInterface -from gprofiler.profilers.registry import get_profilers_registry +from gprofiler.profilers.registry import ( + ProfilerConfig, + get_profilers_registry, + get_runtime_possible_modes, + get_sorted_profilers, +) from gprofiler.spark.sampler import SparkSampler from gprofiler.state import State, init_state from gprofiler.system_metrics import Metrics, NoopSystemMetricsMonitor, SystemMetricsMonitor, SystemMetricsMonitorBase @@ -816,29 +821,32 @@ def parse_cmd_args() -> configargparse.Namespace: def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: - registry = get_profilers_registry() - for name, config in registry.items(): - arg_group = parser.add_argument_group(name) - mode_var = f"{name.lower()}_mode" + for runtime, configs in get_profilers_registry().items(): + arg_group = parser.add_argument_group(runtime) + mode_var = f"{runtime.lower().replace('-', '_')}_mode" + sorted_profilers = get_sorted_profilers(runtime) + # TODO: marcin-ol: organize options and usage for runtime - single source of runtime options? + preferred_profiler = sorted_profilers[0] arg_group.add_argument( - f"--{name.lower()}-mode", + f"--{runtime.lower()}-mode", dest=mode_var, - default=config.default_mode, - help=config.profiler_mode_help, - choices=config.possible_modes, + default=ProfilerConfig.ENABLED_MODE if len(sorted_profilers) > 1 else sorted_profilers[0].default_mode, + help=preferred_profiler.profiler_mode_help, + choices=get_runtime_possible_modes(runtime) + ProfilerConfig.DISABLED_MODES, ) arg_group.add_argument( - f"--no-{name.lower()}", + f"--no-{runtime.lower()}", action="store_const", const="disabled", dest=mode_var, default=True, - help=config.disablement_help, + help=preferred_profiler.disablement_help, ) - for arg in config.profiler_args: - profiler_arg_kwargs = arg.get_dict() - name = profiler_arg_kwargs.pop("name") - arg_group.add_argument(name, **profiler_arg_kwargs) + for config in configs: + for arg in config.profiler_args: + profiler_arg_kwargs = arg.get_dict() + name = profiler_arg_kwargs.pop("name") + arg_group.add_argument(name, **profiler_arg_kwargs) def verify_preconditions(args: configargparse.Namespace, processes_to_profile: Optional[List[Process]]) -> None: diff --git a/gprofiler/profilers/factory.py b/gprofiler/profilers/factory.py index 763a4d2cb..3c704c7d3 100644 --- a/gprofiler/profilers/factory.py +++ b/gprofiler/profilers/factory.py @@ -3,10 +3,9 @@ from gprofiler.log import get_logger_adapter from gprofiler.metadata.system_metadata import get_arch -from gprofiler.platform import is_windows from gprofiler.profilers.perf import SystemProfiler from gprofiler.profilers.profiler_base import NoopProfiler -from gprofiler.profilers.registry import get_profilers_registry +from gprofiler.profilers.registry import ProfilerConfig, get_profilers_registry, get_sorted_profilers if TYPE_CHECKING: from gprofiler.gprofiler_types import UserArgs @@ -24,44 +23,60 @@ def get_profilers( process_profilers_instances: List["ProcessProfilerBase"] = [] system_profiler: Union["SystemProfiler", "NoopProfiler"] = NoopProfiler() - if profiling_mode != "none": - arch = get_arch() - for profiler_name, profiler_config in get_profilers_registry().items(): - lower_profiler_name = profiler_name.lower() - profiler_mode = user_args.get(f"{lower_profiler_name}_mode") - if profiler_mode in ("none", "disabled"): - continue - - supported_archs = ( - profiler_config.supported_windows_archs if is_windows() else profiler_config.supported_archs - ) - if arch not in supported_archs: + if profiling_mode == "none": + return system_profiler, process_profilers_instances + arch = get_arch() + for runtime in get_profilers_registry(): + runtime_args_prefix = runtime.lower() + runtime_mode = user_args.get(f"{runtime_args_prefix}_mode") + if runtime_mode in ProfilerConfig.DISABLED_MODES: + continue + # select configs supporting requested runtime_mode or all configs in order of preference + requested_configs: List[ProfilerConfig] = get_sorted_profilers(runtime) + if runtime_mode != ProfilerConfig.ENABLED_MODE: + requested_configs = [c for c in requested_configs if runtime_mode in c.get_active_modes()] + # select profilers that support this architecture and profiling mode + selected_configs: List[ProfilerConfig] = [] + for config in requested_configs: + profiler_name = config.profiler_name + if arch not in config.get_supported_archs() and len(requested_configs) == 1: logger.warning(f"Disabling {profiler_name} because it doesn't support this architecture ({arch})") continue - - if profiling_mode not in profiler_config.supported_profiling_modes: + if profiling_mode not in config.supported_profiling_modes: logger.warning( f"Disabling {profiler_name} because it doesn't support profiling mode {profiling_mode!r}" ) continue - + selected_configs.append(config) + if not selected_configs: + logger.warning(f"Disabling {runtime} profiling because no profilers were selected") + continue + # create instances of selected profilers one by one, select first that is ready + ready_profiler = None + for profiler_config in selected_configs: + profiler_name = profiler_config.profiler_name profiler_kwargs = profiler_init_kwargs.copy() for key, value in user_args.items(): - if key.startswith(lower_profiler_name) or key in COMMON_PROFILER_ARGUMENT_NAMES: + if key.startswith(runtime_args_prefix) or key in COMMON_PROFILER_ARGUMENT_NAMES: profiler_kwargs[key] = value try: profiler_instance = profiler_config.profiler_class(**profiler_kwargs) + if profiler_instance.check_readiness(): + ready_profiler = profiler_instance + break except Exception: - logger.critical( - f"Couldn't create the {profiler_name} profiler, not continuing." - f" Run with --no-{profiler_name.lower()} to disable this profiler", - exc_info=True, - ) - sys.exit(1) - else: - if isinstance(profiler_instance, SystemProfiler): - system_profiler = profiler_instance - else: - process_profilers_instances.append(profiler_instance) - + if len(requested_configs) == 1: + logger.critical( + f"Couldn't create the {profiler_name} profiler for runtime {runtime}, not continuing." + f" Request different profiler for runtime with --{runtime_args_prefix}-mode, or disable" + f" {runtime} profiling with --no-{runtime_args_prefix} to disable this profiler", + exc_info=True, + ) + sys.exit(1) + if isinstance(ready_profiler, SystemProfiler): + system_profiler = ready_profiler + elif ready_profiler is not None: + process_profilers_instances.append(ready_profiler) + else: + logger.warning(f"Disabling {runtime} profiling because no profilers were ready") return system_profiler, process_profilers_instances diff --git a/gprofiler/profilers/profiler_base.py b/gprofiler/profilers/profiler_base.py index 2c26ffe48..8762eb353 100644 --- a/gprofiler/profilers/profiler_base.py +++ b/gprofiler/profilers/profiler_base.py @@ -48,6 +48,12 @@ def snapshot(self) -> ProcessToProfileData: """ raise NotImplementedError + def check_readiness(self) -> bool: + """ + Check that profiler is ready for use on current platform. + """ + raise NotImplementedError + def stop(self) -> None: pass @@ -99,6 +105,9 @@ def __init__( f"profiling mode: {profiler_state.profiling_mode}" ) + def check_readiness(self) -> bool: + return True + class NoopProfiler(ProfilerInterface): """ @@ -108,6 +117,9 @@ class NoopProfiler(ProfilerInterface): def snapshot(self) -> ProcessToProfileData: return {} + def check_readiness(self) -> bool: + return True + @classmethod def is_noop_profiler(cls, profile_instance: ProfilerInterface) -> bool: return isinstance(profile_instance, cls) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 5d29c611e..55aa4af62 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -407,6 +407,9 @@ def _create_ebpf_profiler( logger.info("Python eBPF profiler initialization failed") return None + def check_readiness(self) -> bool: + return True + def start(self) -> None: if self._ebpf_profiler is not None: self._ebpf_profiler.start() diff --git a/gprofiler/profilers/registry.py b/gprofiler/profilers/registry.py index 9cba93761..a7a72eb16 100644 --- a/gprofiler/profilers/registry.py +++ b/gprofiler/profilers/registry.py @@ -1,39 +1,34 @@ +from collections import defaultdict +from dataclasses import asdict, dataclass from typing import Any, Callable, Dict, List, Optional, Sequence, Type, Union +from gprofiler.metadata.system_metadata import get_arch +from gprofiler.platform import is_windows + +@dataclass class ProfilerArgument: - # TODO convert to a dataclass - def __init__( - self, - name: str, - dest: str, - help: Optional[str] = None, - default: Any = None, - action: Optional[str] = None, - choices: Sequence[Any] = None, - type: Union[Type, Callable[[str], Any]] = None, - metavar: str = None, - const: Any = None, - nargs: str = None, - ): - self.name = name - self.dest = dest - self.help = help - self.default = default - self.action = action - self.choices = choices - self.type = type - self.metavar = metavar - self.const = const - self.nargs = nargs + name: str + dest: str + help: Optional[str] = None + default: Any = None + action: Optional[str] = None + choices: Union[Sequence[Any], None] = None + type: Union[Type, Callable[[str], Any], None] = None + metavar: Optional[str] = None + const: Any = None + nargs: Optional[str] = None def get_dict(self) -> Dict[str, Any]: - return {key: value for key, value in self.__dict__.items() if value is not None} + return {key: value for key, value in asdict(self).items() if value is not None} class ProfilerConfig: def __init__( self, + profiler_name: str, + runtime: str, + is_preferred: bool, profiler_mode_help: Optional[str], disablement_help: Optional[str], profiler_class: Any, @@ -44,6 +39,9 @@ def __init__( default_mode: str = "enabled", arguments: List[ProfilerArgument] = None, ) -> None: + self.profiler_name = profiler_name + self.runtime = runtime + self.is_preferred = is_preferred self.profiler_mode_help = profiler_mode_help self.possible_modes = possible_modes self.supported_archs = supported_archs @@ -54,21 +52,38 @@ def __init__( self.profiler_class = profiler_class self.supported_profiling_modes = supported_profiling_modes + ENABLED_MODE: str = "enabled" + DISABLED_MODES: List[str] = ["disabled", "none"] + + def get_active_modes(self) -> List[str]: + return [ + mode + for mode in self.possible_modes + if mode not in ProfilerConfig.DISABLED_MODES and mode != ProfilerConfig.ENABLED_MODE + ] -profilers_config: Dict[str, ProfilerConfig] = {} + def get_supported_archs(self) -> List[str]: + return self.supported_windows_archs if is_windows() else self.supported_archs + + +profilers_config: Dict[str, List[ProfilerConfig]] = defaultdict(list) def register_profiler( - profiler_name: str, + runtime: str, default_mode: str, possible_modes: List[str], supported_archs: List[str], supported_profiling_modes: List[str], + profiler_name: Optional[str] = None, + is_preferred: bool = False, supported_windows_archs: Optional[List[str]] = None, profiler_mode_argument_help: Optional[str] = None, profiler_arguments: Optional[List[ProfilerArgument]] = None, disablement_help: Optional[str] = None, ) -> Any: + if profiler_name is None: + profiler_name = runtime if profiler_mode_argument_help is None: profiler_mode_argument_help = ( f"Choose the mode for profiling {profiler_name} processes. '{default_mode}'" @@ -80,26 +95,59 @@ def register_profiler( disablement_help = f"Disable the runtime-profiling of {profiler_name} processes" def profiler_decorator(profiler_class: Any) -> Any: - assert profiler_name not in profilers_config, f"{profiler_name} is already registered!" + assert profiler_name is not None, "Profiler name must be defined" + assert profiler_name not in ( + config.profiler_name for profilers in profilers_config.values() for config in profilers + ), f"{profiler_name} is already registered!" assert all( arg.dest.startswith(profiler_name.lower()) for arg in profiler_arguments or [] ), f"{profiler_name}: Profiler args dest must be prefixed with the profiler name" - profilers_config[profiler_name] = ProfilerConfig( - profiler_mode_argument_help, - disablement_help, - profiler_class, - possible_modes, - supported_archs, - supported_profiling_modes, - supported_windows_archs, - default_mode, - profiler_arguments, - ) + profilers_config[runtime] += [ + ProfilerConfig( + profiler_name, + runtime, + is_preferred, + profiler_mode_argument_help, + disablement_help, + profiler_class, + possible_modes, + supported_archs, + supported_profiling_modes, + supported_windows_archs, + default_mode, + profiler_arguments, + ) + ] profiler_class.name = profiler_name return profiler_class return profiler_decorator -def get_profilers_registry() -> Dict[str, ProfilerConfig]: +def get_profilers_registry() -> Dict[str, List[ProfilerConfig]]: return profilers_config + + +def get_profilers_by_name() -> Dict[str, ProfilerConfig]: + return {config.profiler_name: config for configs in profilers_config.values() for config in configs} + + +def get_runtime_possible_modes(runtime: str) -> List[str]: + possible_modes: List[str] = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime]) > 1 else [] + for config in profilers_config[runtime]: + possible_modes += [m for m in config.get_active_modes() if m not in possible_modes] + possible_modes += ProfilerConfig.DISABLED_MODES + return possible_modes + + +def get_sorted_profilers(runtime: str) -> List[ProfilerConfig]: + """ + Get profiler configs sorted by preference. + """ + arch = get_arch() + profiler_configs = sorted( + profilers_config[runtime], + key=lambda c: (arch in c.get_supported_archs(), c.is_preferred, c.profiler_name), + reverse=True, + ) + return profiler_configs diff --git a/tests/test_python.py b/tests/test_python.py index 4321b76b5..a21a5fd8e 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -3,13 +3,18 @@ # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # import os +from pathlib import Path +from typing import List import psutil import pytest +from docker import DockerClient +from docker.models.images import Image from granulate_utils.linux.process import is_musl from gprofiler.profiler_state import ProfilerState -from gprofiler.profilers.python import PythonProfiler +from gprofiler.profilers.python import PySpyProfiler, PythonProfiler +from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from tests.conftest import AssertInCollapsed from tests.utils import ( assert_function_in_collapsed, @@ -17,6 +22,8 @@ is_pattern_in_collapsed, snapshot_pid_collapsed, snapshot_pid_profile, + start_gprofiler_in_container_for_one_session, + wait_for_log, ) @@ -165,3 +172,39 @@ def test_dso_name_in_pyperf_profile( assert insert_dso_name == is_pattern_in_collapsed( rf"{interpreter_frame} \(.+?/libpython{python_version}.*?\.so.*?\)_\[pn\]", collapsed ) + + +@pytest.mark.parametrize( + "runtime,profiler_type,profiler_class_name", + [ + ("python", "py-spy", PySpyProfiler.__name__), + ("python", "pyperf", PythonEbpfProfiler.__name__), + ("python", "auto", PythonEbpfProfiler.__name__), + ("python", "enabled", PythonEbpfProfiler.__name__), + ], +) +def test_select_specific_python_profiler( + docker_client: DockerClient, + gprofiler_docker_image: Image, + output_directory: Path, + output_collapsed: Path, + runtime_specific_args: List[str], + profiler_flags: List[str], + profiler_type: str, + profiler_class_name: str, +) -> None: + """ + Test that correct profiler class is selected as given by --python-mode argument. + """ + if profiler_type == "pyperf" and is_aarch64(): + pytest.xfail("PyPerf doesn't run on Aarch64 - https://github.com/Granulate/gprofiler/issues/499") + elif profiler_type == "enabled": + # make sure the default behavior, with implicit enabled mode leads to auto selection + profiler_flags.remove(f"--python-mode={profiler_type}") + profiler_flags.extend(["--no-perf"]) + gprofiler = start_gprofiler_in_container_for_one_session( + docker_client, gprofiler_docker_image, output_directory, output_collapsed, runtime_specific_args, profiler_flags + ) + wait_for_log(gprofiler, "gProfiler initialized and ready to start profiling", 0, timeout=7) + assert f"Initialized {profiler_class_name}".encode() in gprofiler.logs() + gprofiler.remove(force=True) diff --git a/tests/test_runtime_profilers.py b/tests/test_runtime_profilers.py new file mode 100644 index 000000000..9bb1c3eae --- /dev/null +++ b/tests/test_runtime_profilers.py @@ -0,0 +1,185 @@ +# +# Copyright (c) Granulate. All rights reserved. +# Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. +# +import logging +import sys +from collections import defaultdict +from typing import Any, Dict, Iterable, List, cast + +import pytest +from pytest import MonkeyPatch + +from gprofiler import profilers +from gprofiler.profilers import registry +from gprofiler.profilers.factory import get_profilers +from gprofiler.profilers.profiler_base import NoopProfiler, ProfilerBase +from gprofiler.profilers.registry import ProfilerConfig, register_profiler + + +def _register_mock_profiler( + runtime: str, + profiler_name: str, + profiler_class: Any = NoopProfiler, + is_preferred: bool = False, + default_mode: str = "disabled", + possible_modes: List[str] = ["disabled"], + supported_archs: List[str] = ["x86_64"], + supported_profiling_modes: List[str] = ["cpu"], +) -> None: + register_profiler( + runtime=runtime, + profiler_name=profiler_name, + is_preferred=is_preferred, + default_mode=default_mode, + possible_modes=possible_modes, + supported_archs=supported_archs, + supported_profiling_modes=supported_profiling_modes, + )(profiler_class) + + +def test_profiler_names_are_unique(monkeypatch: MonkeyPatch) -> None: + """ + Make sure that registered profilers are checked for uniqueness. + """ + # register mock class under the same profiler name but for different runtimes; + # define mock modes, to let registration complete. + with monkeypatch.context() as m: + # clear registry before registering mock profilers + m.setattr(profilers.registry, "profilers_config", defaultdict(list)) + _register_mock_profiler("python", profiler_name="mock", possible_modes=["mock-profiler", "disabled"]) + with pytest.raises(AssertionError) as excinfo: + _register_mock_profiler("ruby", profiler_name="mock", possible_modes=["mock-spy", "disabled"]) + assert "mock is already registered" in str(excinfo.value) + + +def _copy_of_registry(keys: Iterable[str] = []) -> Dict[str, List[ProfilerConfig]]: + return defaultdict(list, {k: v[:] for (k, v) in registry.profilers_config.items() if k in keys}) + + +class MockProfiler(ProfilerBase): + def __init__(self, mock_mode: str = "", *args: Any, **kwargs: Any): + logging.warning(f"MOCKINIT/ args={list(*args)}, kwargs={dict(**kwargs)}") + self.mock_mode = mock_mode + + +@pytest.mark.parametrize("profiler_mode", ["mock-perf", "mock-profiler", "disabled"]) +def test_union_of_runtime_profilers_modes( + profiler_mode: str, + monkeypatch: MonkeyPatch, +) -> None: + """ + Test that generated command line arguments allow union of modes from all profilers for a runtime. + """ + with monkeypatch.context() as m: + # clear registry before registering mock profilers; + # keep Perf profiler, as some of its arguments (perf_dwarf_stack_size) are checked in command line parsing + m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + _register_mock_profiler( + "mock", + profiler_name="mock-profiler", + profiler_class=MockProfiler, + possible_modes=["mock-profiler", "disabled"], + ) + MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) + _register_mock_profiler( + "mock", profiler_name="mock-perf", profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"] + ) + from gprofiler.main import parse_cmd_args + + # replace command-line args to include mode we're targeting + m.setattr( + sys, + "argv", + sys.argv[:1] + + [ + "--output-dir", + "./", + "--mock-mode", + profiler_mode, + ], + ) + args = parse_cmd_args() + assert args.mock_mode == profiler_mode + + +@pytest.mark.parametrize("profiler_mode", ["mock-perf", "mock-profiler"]) +def test_select_specific_runtime_profiler( + profiler_mode: str, + monkeypatch: MonkeyPatch, +) -> None: + with monkeypatch.context() as m: + # clear registry before registering mock profilers + m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + _register_mock_profiler( + "mock", + profiler_name="mock-profiler", + profiler_class=MockProfiler, + possible_modes=["mock-profiler", "disabled"], + ) + MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) + + _register_mock_profiler( + "mock", profiler_name="mock-perf", profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"] + ) + print(registry.profilers_config) + from gprofiler.main import parse_cmd_args + + # replace command-line args to include mode we're targeting + m.setattr( + # sys, "argv", sys.argv[:1] + ["--output-dir", "./", f"--{profiler_mode}-mode", profiler_mode, "--no-perf"] + sys, + "argv", + sys.argv[:1] + ["--output-dir", "./", "--mock-mode", profiler_mode, "--no-perf"], + ) + args = parse_cmd_args() + _, process_profilers = get_profilers(args.__dict__) + assert len(process_profilers) == 1 + profiler = process_profilers[0] + assert profiler.__class__.__name__ == {"mock-perf": "MockPerf", "mock-profiler": "MockProfiler"}[profiler_mode] + assert cast(MockProfiler, profiler).mock_mode == profiler_mode + + +@pytest.mark.parametrize("preferred_profiler", ["mock-perf", "mock-profiler"]) +def test_auto_select_preferred_profiler( + preferred_profiler: str, + monkeypatch: MonkeyPatch, +) -> None: + """ + Test that auto selection mechanism correctly selects one of profilers. + """ + with monkeypatch.context() as m: + # clear registry before registering mock profilers + m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + _register_mock_profiler( + "mock", + profiler_name="mock-profiler", + profiler_class=MockProfiler, + is_preferred="mock-profiler" == preferred_profiler, + possible_modes=["mock-profiler", "disabled"], + ) + MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) + + _register_mock_profiler( + "mock", + profiler_name="mock-perf", + profiler_class=MockPerf, + is_preferred="mock-perf" == preferred_profiler, + possible_modes=["mock-perf", "disabled"], + ) + from gprofiler.main import parse_cmd_args + + m.setattr( + sys, + "argv", + sys.argv[:1] + ["--output-dir", "./", "--mock-mode", "enabled", "--no-perf"], + ) + args = parse_cmd_args() + _, process_profilers = get_profilers(args.__dict__) + assert len(process_profilers) == 1 + profiler = process_profilers[0] + assert ( + profiler.__class__.__name__ + == {"mock-perf": "MockPerf", "mock-profiler": "MockProfiler"}[preferred_profiler] + ) + assert cast(MockProfiler, profiler).mock_mode == "enabled" From 5d3b1899a83abe8fc838d0c54f0243b600a1ebb6 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Wed, 21 Jun 2023 09:37:53 +0200 Subject: [PATCH 02/11] Fix computing possible runtime options. --- gprofiler/main.py | 2 +- tests/test_runtime_profilers.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gprofiler/main.py b/gprofiler/main.py index 9f5077f28..a9ec21d2e 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -832,7 +832,7 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: dest=mode_var, default=ProfilerConfig.ENABLED_MODE if len(sorted_profilers) > 1 else sorted_profilers[0].default_mode, help=preferred_profiler.profiler_mode_help, - choices=get_runtime_possible_modes(runtime) + ProfilerConfig.DISABLED_MODES, + choices=get_runtime_possible_modes(runtime), ) arg_group.add_argument( f"--no-{runtime.lower()}", diff --git a/tests/test_runtime_profilers.py b/tests/test_runtime_profilers.py index 9bb1c3eae..5a3d642c1 100644 --- a/tests/test_runtime_profilers.py +++ b/tests/test_runtime_profilers.py @@ -2,7 +2,6 @@ # Copyright (c) Granulate. All rights reserved. # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # -import logging import sys from collections import defaultdict from typing import Any, Dict, Iterable, List, cast @@ -11,6 +10,7 @@ from pytest import MonkeyPatch from gprofiler import profilers +from gprofiler.gprofiler_types import ProcessToProfileData from gprofiler.profilers import registry from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.profiler_base import NoopProfiler, ProfilerBase @@ -59,9 +59,11 @@ def _copy_of_registry(keys: Iterable[str] = []) -> Dict[str, List[ProfilerConfig class MockProfiler(ProfilerBase): def __init__(self, mock_mode: str = "", *args: Any, **kwargs: Any): - logging.warning(f"MOCKINIT/ args={list(*args)}, kwargs={dict(**kwargs)}") self.mock_mode = mock_mode + def snapshot(self) -> ProcessToProfileData: + return {} + @pytest.mark.parametrize("profiler_mode", ["mock-perf", "mock-profiler", "disabled"]) def test_union_of_runtime_profilers_modes( @@ -122,12 +124,10 @@ def test_select_specific_runtime_profiler( _register_mock_profiler( "mock", profiler_name="mock-perf", profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"] ) - print(registry.profilers_config) from gprofiler.main import parse_cmd_args # replace command-line args to include mode we're targeting m.setattr( - # sys, "argv", sys.argv[:1] + ["--output-dir", "./", f"--{profiler_mode}-mode", profiler_mode, "--no-perf"] sys, "argv", sys.argv[:1] + ["--output-dir", "./", "--mock-mode", profiler_mode, "--no-perf"], From c11b6a1eecc9672b642026f7639e3e64e45c9b76 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Wed, 28 Jun 2023 01:41:45 +0200 Subject: [PATCH 03/11] Decouple PySpyProfiler and EbpfProfiler from PythonProfiler --- gprofiler/main.py | 10 +- gprofiler/profilers/__init__.py | 7 +- gprofiler/profilers/factory.py | 18 ++- gprofiler/profilers/python.py | 188 ++++------------------------- gprofiler/profilers/python_ebpf.py | 83 +++++++++++-- gprofiler/profilers/registry.py | 35 +++++- tests/conftest.py | 33 ++++- tests/test_python.py | 34 ++++-- tests/test_runtime_profilers.py | 95 ++++++++++++++- tests/test_sanity.py | 8 +- 10 files changed, 299 insertions(+), 212 deletions(-) diff --git a/gprofiler/main.py b/gprofiler/main.py index a9ec21d2e..4ad41c0fb 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -50,10 +50,9 @@ from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.profiler_base import NoopProfiler, ProcessProfilerBase, ProfilerInterface from gprofiler.profilers.registry import ( - ProfilerConfig, + get_preferred_or_first_profiler, get_profilers_registry, get_runtime_possible_modes, - get_sorted_profilers, ) from gprofiler.spark.sampler import SparkSampler from gprofiler.state import State, init_state @@ -824,13 +823,12 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: for runtime, configs in get_profilers_registry().items(): arg_group = parser.add_argument_group(runtime) mode_var = f"{runtime.lower().replace('-', '_')}_mode" - sorted_profilers = get_sorted_profilers(runtime) - # TODO: marcin-ol: organize options and usage for runtime - single source of runtime options? - preferred_profiler = sorted_profilers[0] + # TODO: organize options and usage for runtime - single source of runtime options? + preferred_profiler = get_preferred_or_first_profiler(runtime) arg_group.add_argument( f"--{runtime.lower()}-mode", dest=mode_var, - default=ProfilerConfig.ENABLED_MODE if len(sorted_profilers) > 1 else sorted_profilers[0].default_mode, + default=preferred_profiler.default_mode, help=preferred_profiler.profiler_mode_help, choices=get_runtime_possible_modes(runtime), ) diff --git a/gprofiler/profilers/__init__.py b/gprofiler/profilers/__init__.py index 295a83f19..4d9602d45 100644 --- a/gprofiler/profilers/__init__.py +++ b/gprofiler/profilers/__init__.py @@ -1,17 +1,18 @@ # NOTE: Make sure to import any new process profilers to load it from gprofiler.platform import is_linux from gprofiler.profilers.dotnet import DotnetProfiler -from gprofiler.profilers.python import PythonProfiler +from gprofiler.profilers.python import PySpyProfiler if is_linux(): from gprofiler.profilers.java import JavaProfiler from gprofiler.profilers.perf import SystemProfiler from gprofiler.profilers.php import PHPSpyProfiler + from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from gprofiler.profilers.ruby import RbSpyProfiler -__all__ = ["PythonProfiler", "DotnetProfiler"] +__all__ = ["PySpyProfiler", "DotnetProfiler"] if is_linux(): - __all__ += ["JavaProfiler", "PHPSpyProfiler", "RbSpyProfiler", "SystemProfiler"] + __all__ += ["JavaProfiler", "PHPSpyProfiler", "RbSpyProfiler", "SystemProfiler", "PythonEbpfProfiler"] del is_linux diff --git a/gprofiler/profilers/factory.py b/gprofiler/profilers/factory.py index 3c704c7d3..b6d93e933 100644 --- a/gprofiler/profilers/factory.py +++ b/gprofiler/profilers/factory.py @@ -5,7 +5,12 @@ from gprofiler.metadata.system_metadata import get_arch from gprofiler.profilers.perf import SystemProfiler from gprofiler.profilers.profiler_base import NoopProfiler -from gprofiler.profilers.registry import ProfilerConfig, get_profilers_registry, get_sorted_profilers +from gprofiler.profilers.registry import ( + ProfilerConfig, + get_profiler_arguments, + get_profilers_registry, + get_sorted_profilers, +) if TYPE_CHECKING: from gprofiler.gprofiler_types import UserArgs @@ -27,7 +32,7 @@ def get_profilers( return system_profiler, process_profilers_instances arch = get_arch() for runtime in get_profilers_registry(): - runtime_args_prefix = runtime.lower() + runtime_args_prefix = runtime.lower().replace("-", "_") runtime_mode = user_args.get(f"{runtime_args_prefix}_mode") if runtime_mode in ProfilerConfig.DISABLED_MODES: continue @@ -53,11 +58,18 @@ def get_profilers( continue # create instances of selected profilers one by one, select first that is ready ready_profiler = None + runtime_arg_names = [arg.dest for config in get_profilers_registry()[runtime] for arg in config.profiler_args] for profiler_config in selected_configs: profiler_name = profiler_config.profiler_name profiler_kwargs = profiler_init_kwargs.copy() + profiler_arg_names = [arg.dest for arg in get_profiler_arguments(runtime, profiler_name)] for key, value in user_args.items(): - if key.startswith(runtime_args_prefix) or key in COMMON_PROFILER_ARGUMENT_NAMES: + if ( + key in profiler_arg_names + or key in COMMON_PROFILER_ARGUMENT_NAMES + or key.startswith(runtime_args_prefix) + and key not in runtime_arg_names + ): profiler_kwargs[key] = value try: profiler_instance = profiler_config.profiler_class(**profiler_kwargs) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 55aa4af62..43573a86b 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -27,28 +27,17 @@ ProcessStoppedException, StopEventSetException, ) -from gprofiler.gprofiler_types import ( - ProcessToProfileData, - ProcessToStackSampleCounters, - ProfileData, - StackToSampleCount, - nonnegative_integer, -) +from gprofiler.gprofiler_types import ProcessToStackSampleCounters, ProfileData, StackToSampleCount from gprofiler.log import get_logger_adapter from gprofiler.metadata import application_identifiers from gprofiler.metadata.application_metadata import ApplicationMetadata from gprofiler.metadata.py_module_version import get_modules_versions -from gprofiler.metadata.system_metadata import get_arch from gprofiler.platform import is_linux, is_windows from gprofiler.profiler_state import ProfilerState -from gprofiler.profilers.profiler_base import ProfilerInterface, SpawningProcessProfilerBase -from gprofiler.profilers.registry import ProfilerArgument, register_profiler -from gprofiler.utils.collapsed_format import parse_one_collapsed_file - -if is_linux(): - from gprofiler.profilers.python_ebpf import PythonEbpfProfiler, PythonEbpfError - +from gprofiler.profilers.profiler_base import SpawningProcessProfilerBase +from gprofiler.profilers.registry import register_profiler from gprofiler.utils import pgrep_exe, pgrep_maps, random_prefix, removed_path, resource_path, run_process +from gprofiler.utils.collapsed_format import parse_one_collapsed_file from gprofiler.utils.process import process_comm, search_proc_maps logger = get_logger_adapter(__name__) @@ -163,6 +152,19 @@ def make_application_metadata(self, process: Process) -> Dict[str, Any]: return metadata +@register_profiler( + "Python", + profiler_name="PySpy", + # py-spy is like pyspy, it's confusing and I mix between them + possible_modes=["auto", "pyspy", "py-spy"], + default_mode="auto", + # we build pyspy for both,. + supported_archs=["x86_64", "aarch64"], + supported_windows_archs=["AMD64"], + # profiler arguments are defined by preferred profiler of the runtime, that is PythonEbpfProfiler + profiler_arguments=[], + supported_profiling_modes=["cpu"], +) class PySpyProfiler(SpawningProcessProfilerBase): MAX_FREQUENCY = 50 _EXTRA_TIMEOUT = 10 # give py-spy some seconds to run (added to the duration) @@ -173,10 +175,14 @@ def __init__( duration: int, profiler_state: ProfilerState, *, - add_versions: bool, + python_mode: str, + python_add_versions: bool, ): super().__init__(frequency, duration, profiler_state) - self.add_versions = add_versions + if python_mode == "py-spy": + python_mode = "pyspy" + assert python_mode in ("auto", "pyspy"), f"unexpected mode: {python_mode}" + self.add_versions = python_add_versions self._metadata = PythonMetadata(self._profiler_state.stop_event) def _make_command(self, pid: int, output_path: str, duration: int) -> List[str]: @@ -289,153 +295,5 @@ def _should_skip_process(self, process: Process) -> bool: return False - -@register_profiler( - "Python", - # py-spy is like pyspy, it's confusing and I mix between them - possible_modes=["auto", "pyperf", "pyspy", "py-spy", "disabled"], - default_mode="auto", - # we build pyspy for both, pyperf only for x86_64. - # TODO: this inconsistency shows that py-spy and pyperf should have different Profiler classes, - # we should split them in the future. - supported_archs=["x86_64", "aarch64"], - supported_windows_archs=["AMD64"], - profiler_mode_argument_help="Select the Python profiling mode: auto (try PyPerf, resort to py-spy if it fails), " - "pyspy (always use py-spy), pyperf (always use PyPerf, and avoid py-spy even if it fails)" - " or disabled (no runtime profilers for Python).", - profiler_arguments=[ - # TODO should be prefixed with --python- - ProfilerArgument( - "--no-python-versions", - dest="python_add_versions", - action="store_false", - default=True, - help="Don't add version information to Python frames. If not set, frames from packages are displayed with " - "the name of the package and its version, and frames from Python built-in modules are displayed with " - "Python's full version.", - ), - # TODO should be prefixed with --python- - ProfilerArgument( - "--pyperf-user-stacks-pages", - dest="python_pyperf_user_stacks_pages", - default=None, - type=nonnegative_integer, - help="Number of user stack-pages that PyPerf will collect, this controls the maximum stack depth of native " - "user frames. Pass 0 to disable user native stacks altogether.", - ), - ProfilerArgument( - "--python-pyperf-verbose", - dest="python_pyperf_verbose", - action="store_true", - help="Enable PyPerf in verbose mode (max verbosity)", - ), - ], - supported_profiling_modes=["cpu"], -) -class PythonProfiler(ProfilerInterface): - """ - Controls PySpyProfiler & PythonEbpfProfiler as needed, providing a clean interface - to GProfiler. - """ - - def __init__( - self, - frequency: int, - duration: int, - profiler_state: ProfilerState, - python_mode: str, - python_add_versions: bool, - python_pyperf_user_stacks_pages: Optional[int], - python_pyperf_verbose: bool, - ): - if python_mode == "py-spy": - python_mode = "pyspy" - - assert python_mode in ("auto", "pyperf", "pyspy"), f"unexpected mode: {python_mode}" - - if get_arch() != "x86_64" or is_windows(): - if python_mode == "pyperf": - raise Exception(f"PyPerf is supported only on x86_64 (and not on this arch {get_arch()})") - python_mode = "pyspy" - - if python_mode in ("auto", "pyperf"): - self._ebpf_profiler = self._create_ebpf_profiler( - frequency, - duration, - profiler_state, - python_add_versions, - python_pyperf_user_stacks_pages, - python_pyperf_verbose, - ) - else: - self._ebpf_profiler = None - - if python_mode == "pyspy" or (self._ebpf_profiler is None and python_mode == "auto"): - self._pyspy_profiler: Optional[PySpyProfiler] = PySpyProfiler( - frequency, - duration, - profiler_state, - add_versions=python_add_versions, - ) - else: - self._pyspy_profiler = None - - if is_linux(): - - def _create_ebpf_profiler( - self, - frequency: int, - duration: int, - profiler_state: ProfilerState, - add_versions: bool, - user_stacks_pages: Optional[int], - verbose: bool, - ) -> Optional[PythonEbpfProfiler]: - try: - profiler = PythonEbpfProfiler( - frequency, - duration, - profiler_state, - add_versions=add_versions, - user_stacks_pages=user_stacks_pages, - verbose=verbose, - ) - profiler.test() - return profiler - except Exception as e: - logger.debug(f"eBPF profiler error: {str(e)}") - logger.info("Python eBPF profiler initialization failed") - return None - def check_readiness(self) -> bool: return True - - def start(self) -> None: - if self._ebpf_profiler is not None: - self._ebpf_profiler.start() - elif self._pyspy_profiler is not None: - self._pyspy_profiler.start() - - def snapshot(self) -> ProcessToProfileData: - if self._ebpf_profiler is not None: - try: - return self._ebpf_profiler.snapshot() - except PythonEbpfError as e: - assert not self._ebpf_profiler.is_running() - logger.warning( - "Python eBPF profiler failed, restarting PyPerf...", - pyperf_exit_code=e.returncode, - pyperf_stdout=e.stdout, - pyperf_stderr=e.stderr, - ) - self._ebpf_profiler.start() - return {} # empty this round - else: - assert self._pyspy_profiler is not None - return self._pyspy_profiler.snapshot() - - def stop(self) -> None: - if self._ebpf_profiler is not None: - self._ebpf_profiler.stop() - elif self._pyspy_profiler is not None: - self._pyspy_profiler.stop() diff --git a/gprofiler/profilers/python_ebpf.py b/gprofiler/profilers/python_ebpf.py index a331b97df..4954fe4a0 100644 --- a/gprofiler/profilers/python_ebpf.py +++ b/gprofiler/profilers/python_ebpf.py @@ -14,12 +14,13 @@ from psutil import NoSuchProcess, Process from gprofiler.exceptions import CalledProcessError, StopEventSetException -from gprofiler.gprofiler_types import ProcessToProfileData, ProfileData +from gprofiler.gprofiler_types import ProcessToProfileData, ProfileData, nonnegative_integer from gprofiler.log import get_logger_adapter from gprofiler.metadata import application_identifiers from gprofiler.profiler_state import ProfilerState from gprofiler.profilers import python from gprofiler.profilers.profiler_base import ProfilerBase +from gprofiler.profilers.registry import ProfilerArgument, register_profiler from gprofiler.utils import ( poll_process, random_prefix, @@ -41,6 +42,47 @@ class PythonEbpfError(CalledProcessError): """ +@register_profiler( + "Python", + profiler_name="PyPerf", + is_preferred=True, + # py-spy is like pyspy, it's confusing and I mix between them + possible_modes=["auto", "pyperf"], + default_mode="auto", + # we build pyperf only for x86_64. + supported_archs=["x86_64"], + profiler_mode_argument_help="Select the Python profiling mode: auto (try PyPerf, resort to py-spy if it fails), " + "pyspy (always use py-spy), pyperf (always use PyPerf, and avoid py-spy even if it fails)" + " or disabled (no runtime profilers for Python).", + profiler_arguments=[ + # TODO should be prefixed with --python- + ProfilerArgument( + "--no-python-versions", + dest="python_add_versions", + action="store_false", + default=True, + help="Don't add version information to Python frames. If not set, frames from packages are displayed with " + "the name of the package and its version, and frames from Python built-in modules are displayed with " + "Python's full version.", + ), + # TODO should be prefixed with --python- + ProfilerArgument( + "--pyperf-user-stacks-pages", + dest="python_pyperf_user_stacks_pages", + default=None, + type=nonnegative_integer, + help="Number of user stack-pages that PyPerf will collect, this controls the maximum stack depth of native " + "user frames. Pass 0 to disable user native stacks altogether.", + ), + ProfilerArgument( + "--python-pyperf-verbose", + dest="python_pyperf_verbose", + action="store_true", + help="Enable PyPerf in verbose mode (max verbosity)", + ), + ], + supported_profiling_modes=["cpu"], +) class PythonEbpfProfiler(ProfilerBase): MAX_FREQUENCY = 1000 PYPERF_RESOURCE = "python/pyperf/PyPerf" @@ -61,18 +103,20 @@ def __init__( duration: int, profiler_state: ProfilerState, *, - add_versions: bool, - user_stacks_pages: Optional[int] = None, - verbose: bool, + python_mode: str, + python_add_versions: bool, + python_pyperf_user_stacks_pages: Optional[int] = None, + python_pyperf_verbose: bool, ): super().__init__(frequency, duration, profiler_state) + assert python_mode in ("auto", "pyperf"), f"unexpected mode: {python_mode}" self.process: Optional[Popen] = None self.output_path = Path(self._profiler_state.storage_dir) / f"pyperf.{random_prefix()}.col" - self.add_versions = add_versions - self.user_stacks_pages = user_stacks_pages + self.add_versions = python_add_versions + self.user_stacks_pages = python_pyperf_user_stacks_pages self._kernel_offsets: Dict[str, int] = {} self._metadata = python.PythonMetadata(self._profiler_state.stop_event) - self._verbose = verbose + self._verbose = python_pyperf_verbose @classmethod def _check_output(cls, process: Popen, output_path: Path) -> None: @@ -230,7 +274,16 @@ def _dump(self) -> Path: ), process.args # mypy raise PythonEbpfError(exit_status, process.args, stdout, stderr) - def snapshot(self) -> ProcessToProfileData: + def check_readiness(self) -> bool: + try: + self.test() + return True + except Exception as e: + logger.debug(f"eBPF profiler error: {str(e)}") + logger.info("Python eBPF profiler cannot be used on this system") + return False + + def _ebpf_snapshot(self) -> ProcessToProfileData: if self._profiler_state.stop_event.wait(self._duration): raise StopEventSetException() collapsed_path = self._dump() @@ -262,6 +315,20 @@ def snapshot(self) -> ProcessToProfileData: profiles[pid] = ProfileData(parsed[pid], appid, app_metadata, container_name) return profiles + def snapshot(self) -> ProcessToProfileData: + try: + return self._ebpf_snapshot() + except PythonEbpfError as e: + assert not self.is_running() + logger.warning( + "Python eBPF profiler failed, restarting PyPerf...", + pyperf_exit_code=e.returncode, + pyperf_stdout=e.stdout, + pyperf_stderr=e.stderr, + ) + self.start() + return {} # empty this round + def _terminate(self) -> Tuple[Optional[int], str, str]: if self.is_running(): assert self.process is not None # for mypy diff --git a/gprofiler/profilers/registry.py b/gprofiler/profilers/registry.py index a7a72eb16..ebd50b178 100644 --- a/gprofiler/profilers/registry.py +++ b/gprofiler/profilers/registry.py @@ -100,8 +100,8 @@ def profiler_decorator(profiler_class: Any) -> Any: config.profiler_name for profilers in profilers_config.values() for config in profilers ), f"{profiler_name} is already registered!" assert all( - arg.dest.startswith(profiler_name.lower()) for arg in profiler_arguments or [] - ), f"{profiler_name}: Profiler args dest must be prefixed with the profiler name" + arg.dest.startswith(runtime.lower()) for arg in profiler_arguments or [] + ), f"{profiler_name}: Profiler args dest must be prefixed with the profiler runtime name" profilers_config[runtime] += [ ProfilerConfig( profiler_name, @@ -142,7 +142,7 @@ def get_runtime_possible_modes(runtime: str) -> List[str]: def get_sorted_profilers(runtime: str) -> List[ProfilerConfig]: """ - Get profiler configs sorted by preference. + Get all profiler configs registered for given runtime sorted by preference. """ arch = get_arch() profiler_configs = sorted( @@ -151,3 +151,32 @@ def get_sorted_profilers(runtime: str) -> List[ProfilerConfig]: reverse=True, ) return profiler_configs + + +def get_preferred_or_first_profiler(runtime: str) -> ProfilerConfig: + return next(filter(lambda config: config.is_preferred, profilers_config[runtime]), profilers_config[runtime][0]) + + +def get_profiler_arguments(runtime: str, profiler_name: str) -> List[ProfilerArgument]: + """ + For now the common and specific profiler command-line arguments are defined together, at profiler + registration. + Once we implement a mechanism to hold runtime-wide options (including arguments), this function will be + obsolete. + """ + # Arguments can be distinguished by prefix of their dest variable name. + # Group all profiler arguments and exclude those that are prefixed with other profiler names. + runtime_lower = runtime.lower() + other_profiler_prefixes = [ + f"{runtime_lower}_{config.profiler_name.lower().replace('-', '_')}" + for config in profilers_config[runtime] + if config.profiler_name != profiler_name + ] + all_runtime_args: List[ProfilerArgument] = [ + arg + for config in profilers_config[runtime] + for arg in config.profiler_args + if arg.dest.startswith(runtime_lower) + ] + profiler_args = [arg for arg in all_runtime_args if False is any(map(arg.dest.startswith, other_profiler_prefixes))] + return profiler_args diff --git a/tests/conftest.py b/tests/conftest.py index 6f87331f0..5aa3a9ef8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,7 @@ import secrets import stat import subprocess +import sys from contextlib import _GeneratorContextManager, contextmanager from functools import lru_cache, partial from pathlib import Path @@ -19,7 +20,7 @@ from docker.models.containers import Container from docker.models.images import Image from psutil import Process -from pytest import FixtureRequest, TempPathFactory, fixture +from pytest import FixtureRequest, MonkeyPatch, TempPathFactory, fixture from gprofiler.consts import CPU_PROFILING_MODE from gprofiler.containers_client import ContainerNamesClient @@ -34,7 +35,9 @@ ) from gprofiler.metadata.enrichment import EnrichmentOptions from gprofiler.profiler_state import ProfilerState +from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.java import AsyncProfiledProcess, JattachJcmdRunner +from gprofiler.profilers.profiler_base import ProfilerBase from gprofiler.state import init_state from tests import CONTAINERS_DIRECTORY, PHPSPY_DURATION from tests.utils import ( @@ -580,6 +583,33 @@ def profiler_flags(runtime: str, profiler_type: str) -> List[str]: return flags +@fixture +def make_profiler_instance( + profiler_flags: List[str], monkeypatch: MonkeyPatch, output_directory: Path, profiler_state: ProfilerState +) -> Callable[[], _GeneratorContextManager]: + """ + Deliver an instance of profiler selected by runtime and profiler_type. + """ + + @contextmanager + def _make_profiler_instance() -> Iterator[ProfilerBase]: + with monkeypatch.context() as m: + from gprofiler.main import parse_cmd_args + + m.setattr( + sys, + "argv", + sys.argv[:1] + ["--output-dir", str(output_directory), "--no-perf"] + profiler_flags, + ) + args = parse_cmd_args() + _, process_profilers = get_profilers(args.__dict__, profiler_state=profiler_state) + assert len(process_profilers) == 1 + profiler = process_profilers[0] + yield cast(ProfilerBase, profiler) + + return _make_profiler_instance + + @fixture(autouse=True, scope="session") def _init_profiler() -> None: """ @@ -626,7 +656,6 @@ def python_version(in_container: bool, application_docker_container: Container) else: # If not running in a container the test application runs on host output = subprocess.check_output("python3 --version", stderr=subprocess.STDOUT, shell=True) - # Output is expected to look like e.g. "Python 3.9.7" return cast(str, output.decode().strip().split()[-1]) diff --git a/tests/test_python.py b/tests/test_python.py index a21a5fd8e..e843052c1 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -3,8 +3,9 @@ # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # import os +from contextlib import _GeneratorContextManager from pathlib import Path -from typing import List +from typing import Callable, List import psutil import pytest @@ -12,8 +13,7 @@ from docker.models.images import Image from granulate_utils.linux.process import is_musl -from gprofiler.profiler_state import ProfilerState -from gprofiler.profilers.python import PySpyProfiler, PythonProfiler +from gprofiler.profilers.python import PySpyProfiler from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from tests.conftest import AssertInCollapsed from tests.utils import ( @@ -34,10 +34,12 @@ def runtime() -> str: @pytest.mark.parametrize("in_container", [True]) @pytest.mark.parametrize("application_image_tag", ["libpython"]) +@pytest.mark.parametrize("profiler_type", ["pyspy"]) def test_python_select_by_libpython( application_pid: int, assert_collapsed: AssertInCollapsed, - profiler_state: ProfilerState, + profiler_flags: List[str], + make_profiler_instance: Callable[[], _GeneratorContextManager], ) -> None: """ Tests that profiling of processes running Python, whose basename(readlink("/proc/pid/exe")) isn't "python" @@ -45,8 +47,10 @@ def test_python_select_by_libpython( We expect to select these because they have "libpython" in their "/proc/pid/maps". This test runs a Python named "shmython". """ - with PythonProfiler(1000, 1, profiler_state, "pyspy", True, None, False) as profiler: - process_collapsed = snapshot_pid_collapsed(profiler, application_pid) + profiler_flags.extend(["-f", "1000", "-d", "1"]) + with make_profiler_instance() as profiler: + with profiler: + process_collapsed = snapshot_pid_collapsed(profiler, application_pid) assert_collapsed(process_collapsed) assert all(stack.startswith("shmython") for stack in process_collapsed.keys()) @@ -83,7 +87,8 @@ def test_python_matrix( assert_collapsed: AssertInCollapsed, profiler_type: str, application_image_tag: str, - profiler_state: ProfilerState, + profiler_flags: List[str], + make_profiler_instance: Callable[[], _GeneratorContextManager], ) -> None: python_version, libc, app = application_image_tag.split("-") @@ -105,8 +110,10 @@ def test_python_matrix( if python_version in ["3.7", "3.8", "3.9", "3.10", "3.11"] and profiler_type == "py-spy" and libc == "musl": pytest.xfail("This combination fails, see https://github.com/Granulate/gprofiler/issues/714") - with PythonProfiler(1000, 2, profiler_state, profiler_type, True, None, False) as profiler: - profile = snapshot_pid_profile(profiler, application_pid) + profiler_flags.extend(["-f", "1000", "-d", "2"]) + with make_profiler_instance() as profiler: + with profiler: + profile = snapshot_pid_profile(profiler, application_pid) collapsed = profile.stacks @@ -155,15 +162,18 @@ def test_dso_name_in_pyperf_profile( profiler_type: str, application_image_tag: str, insert_dso_name: bool, - profiler_state: ProfilerState, + profiler_flags: List[str], + make_profiler_instance: Callable[[], _GeneratorContextManager], ) -> None: if is_aarch64() and profiler_type == "pyperf": pytest.skip( "PyPerf doesn't support aarch64 architecture, see https://github.com/Granulate/gprofiler/issues/499" ) - with PythonProfiler(1000, 2, profiler_state, profiler_type, True, None, False) as profiler: - profile = snapshot_pid_profile(profiler, application_pid) + profiler_flags.extend(["-f", "1000", "-d", "2"]) + with make_profiler_instance() as profiler: + with profiler: + profile = snapshot_pid_profile(profiler, application_pid) python_version, _, _ = application_image_tag.split("-") interpreter_frame = "PyEval_EvalFrameEx" if python_version == "2.7" else "_PyEval_EvalFrameDefault" collapsed = profile.stacks diff --git a/tests/test_runtime_profilers.py b/tests/test_runtime_profilers.py index 5a3d642c1..c6dc9e319 100644 --- a/tests/test_runtime_profilers.py +++ b/tests/test_runtime_profilers.py @@ -4,7 +4,7 @@ # import sys from collections import defaultdict -from typing import Any, Dict, Iterable, List, cast +from typing import Any, Dict, Iterable, List, Optional, Set, cast import pytest from pytest import MonkeyPatch @@ -14,7 +14,7 @@ from gprofiler.profilers import registry from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.profiler_base import NoopProfiler, ProfilerBase -from gprofiler.profilers.registry import ProfilerConfig, register_profiler +from gprofiler.profilers.registry import ProfilerArgument, ProfilerConfig, register_profiler def _register_mock_profiler( @@ -26,6 +26,7 @@ def _register_mock_profiler( possible_modes: List[str] = ["disabled"], supported_archs: List[str] = ["x86_64"], supported_profiling_modes: List[str] = ["cpu"], + profiler_arguments: Optional[List[ProfilerArgument]] = None, ) -> None: register_profiler( runtime=runtime, @@ -35,9 +36,14 @@ def _register_mock_profiler( possible_modes=possible_modes, supported_archs=supported_archs, supported_profiling_modes=supported_profiling_modes, + profiler_arguments=profiler_arguments, )(profiler_class) +def _copy_of_registry(keys: Iterable[str] = []) -> Dict[str, List[ProfilerConfig]]: + return defaultdict(list, {k: v[:] for (k, v) in registry.profilers_config.items() if k in keys}) + + def test_profiler_names_are_unique(monkeypatch: MonkeyPatch) -> None: """ Make sure that registered profilers are checked for uniqueness. @@ -53,13 +59,10 @@ def test_profiler_names_are_unique(monkeypatch: MonkeyPatch) -> None: assert "mock is already registered" in str(excinfo.value) -def _copy_of_registry(keys: Iterable[str] = []) -> Dict[str, List[ProfilerConfig]]: - return defaultdict(list, {k: v[:] for (k, v) in registry.profilers_config.items() if k in keys}) - - class MockProfiler(ProfilerBase): def __init__(self, mock_mode: str = "", *args: Any, **kwargs: Any): self.mock_mode = mock_mode + self.kwargs = dict(**kwargs) def snapshot(self) -> ProcessToProfileData: return {} @@ -183,3 +186,83 @@ def test_auto_select_preferred_profiler( == {"mock-perf": "MockPerf", "mock-profiler": "MockProfiler"}[preferred_profiler] ) assert cast(MockProfiler, profiler).mock_mode == "enabled" + + +@pytest.mark.parametrize( + "preferred_profiler,expected_args,unwanted_args", + [ + pytest.param( + "mock-perf", + {"duration", "frequency", "mock_one", "mock_two", "mock_mock_perf_two"}, + {"mock_mock_profiler_one"}, + id="mock_perf", + ), + pytest.param( + "mock-profiler", + {"duration", "frequency", "mock_one", "mock_two", "mock_mock_profiler_one"}, + {"mock_mock_perf_two"}, + id="mock_profiler", + ), + ], +) +def test_assign_correct_profiler_arguments( + monkeypatch: MonkeyPatch, + preferred_profiler: str, + expected_args: Set[str], + unwanted_args: Set[str], +) -> None: + """ + Test that selected profiler gets all of its own or common arguments, none from other profiler. + """ + with monkeypatch.context() as m: + # clear registry before registering mock profilers + m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + _register_mock_profiler( + "mock", + profiler_name="mock-profiler", + profiler_class=MockProfiler, + is_preferred="mock-profiler" == preferred_profiler, + possible_modes=["mock-profiler", "disabled"], + profiler_arguments=[ + ProfilerArgument("--mock-common-one", "mock_one"), + ProfilerArgument("--mock-mock-profiler-one", "mock_mock_profiler_one"), + ], + ) + MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) + + _register_mock_profiler( + "mock", + profiler_name="mock-perf", + profiler_class=MockPerf, + is_preferred="mock-perf" == preferred_profiler, + possible_modes=["mock-perf", "disabled"], + profiler_arguments=[ + ProfilerArgument("--mock-common-two", "mock_two"), + ProfilerArgument("--mock-mock-perf-two", "mock_mock_perf_two"), + ], + ) + from gprofiler.main import parse_cmd_args + + m.setattr( + sys, + "argv", + sys.argv[:1] + + [ + "--output-dir", + "./", + "--mock-mode", + "enabled", + "--no-perf", + "--mock-common-one=check", + "--mock-common-two=check", + "--mock-mock-profiler-one=check", + "--mock-mock-perf-two=check", + ], + ) + args = parse_cmd_args() + _, process_profilers = get_profilers(args.__dict__) + assert len(process_profilers) == 1 + profiler = process_profilers[0] + mock = cast(MockProfiler, profiler) + assert set(mock.kwargs.keys()).intersection(unwanted_args) == set() + assert set(mock.kwargs.keys()) == expected_args diff --git a/tests/test_sanity.py b/tests/test_sanity.py index 9f87c2557..f79b236e2 100644 --- a/tests/test_sanity.py +++ b/tests/test_sanity.py @@ -65,7 +65,7 @@ def test_pyspy( profiler_state: ProfilerState, ) -> None: _ = assert_app_id # Required for mypy unused argument warning - with PySpyProfiler(1000, 3, profiler_state, add_versions=True) as profiler: + with PySpyProfiler(1000, 3, profiler_state, python_mode="pyspy", python_add_versions=True) as profiler: # not using snapshot_one_collapsed because there are multiple Python processes running usually. process_collapsed = snapshot_pid_collapsed(profiler, application_pid) assert_collapsed(process_collapsed) @@ -157,7 +157,9 @@ def test_python_ebpf( ) _ = assert_app_id # Required for mypy unused argument warning - with PythonEbpfProfiler(1000, 5, profiler_state, add_versions=True, verbose=False) as profiler: + with PythonEbpfProfiler( + 1000, 5, profiler_state, python_mode="pyperf", python_add_versions=True, python_pyperf_verbose=False + ) as profiler: try: process_collapsed = snapshot_pid_collapsed(profiler, application_pid) except UnicodeDecodeError as e: @@ -263,7 +265,6 @@ def test_container_name_when_stopped( output_directory: Path, output_collapsed: Path, runtime_specific_args: List[str], - profiler_type: str, profiler_flags: List[str], application_docker_container: Container, ) -> None: @@ -303,7 +304,6 @@ def test_profiling_provided_pids( runtime_specific_args: List[str], profiler_flags: List[str], application_factory: Callable[[], _GeneratorContextManager], - profiler_type: str, ) -> None: """ Tests that gprofiler will profile only processes provided via flag --pids From 36f81aa956cd74303bd09d8f6ebe0eb3c8982470 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Mon, 3 Jul 2023 17:51:45 +0200 Subject: [PATCH 04/11] Address comments in review --- tests/conftest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 5aa3a9ef8..f1a73361a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -591,17 +591,21 @@ def make_profiler_instance( Deliver an instance of profiler selected by runtime and profiler_type. """ + # Reuse gProfiler flow to initialize specific profiler instance - one selected by runtime fixture. @contextmanager def _make_profiler_instance() -> Iterator[ProfilerBase]: with monkeypatch.context() as m: from gprofiler.main import parse_cmd_args + # overide command-line args to enable only one process profiler m.setattr( sys, "argv", sys.argv[:1] + ["--output-dir", str(output_directory), "--no-perf"] + profiler_flags, ) + # invoke gProfiler command-line parsing to collect arguments args = parse_cmd_args() + # call factory to deliver initialized profiler instance _, process_profilers = get_profilers(args.__dict__, profiler_state=profiler_state) assert len(process_profilers) == 1 profiler = process_profilers[0] From 43f66287001d2606e5c355f457c5d0d02024a7a0 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Tue, 11 Jul 2023 14:36:21 +0200 Subject: [PATCH 05/11] Address comments in review, 2 --- gprofiler/main.py | 23 +++++++---- gprofiler/profilers/factory.py | 11 ++---- gprofiler/profilers/python.py | 1 - gprofiler/profilers/python_ebpf.py | 1 - gprofiler/profilers/registry.py | 22 ++++++----- tests/test_python.py | 61 +++++++++++++++++------------- tests/utils.py | 30 ++++++++++++++- 7 files changed, 97 insertions(+), 52 deletions(-) diff --git a/gprofiler/main.py b/gprofiler/main.py index 4ad41c0fb..9c2f08b20 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -51,8 +51,10 @@ from gprofiler.profilers.profiler_base import NoopProfiler, ProcessProfilerBase, ProfilerInterface from gprofiler.profilers.registry import ( get_preferred_or_first_profiler, + get_profiler_arguments, get_profilers_registry, get_runtime_possible_modes, + get_sorted_profilers, ) from gprofiler.spark.sampler import SparkSampler from gprofiler.state import State, init_state @@ -820,9 +822,11 @@ def parse_cmd_args() -> configargparse.Namespace: def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: - for runtime, configs in get_profilers_registry().items(): + # add command-line arguments for each profiling runtime, but only for profilers that are working + # with current architecture. + for runtime in get_profilers_registry(): arg_group = parser.add_argument_group(runtime) - mode_var = f"{runtime.lower().replace('-', '_')}_mode" + mode_var = f"{runtime.lower()}_mode" # TODO: organize options and usage for runtime - single source of runtime options? preferred_profiler = get_preferred_or_first_profiler(runtime) arg_group.add_argument( @@ -840,11 +844,16 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: default=True, help=preferred_profiler.disablement_help, ) - for config in configs: - for arg in config.profiler_args: - profiler_arg_kwargs = arg.get_dict() - name = profiler_arg_kwargs.pop("name") - arg_group.add_argument(name, **profiler_arg_kwargs) + # select arguments from sorted filtered list of profilers in current runtime + sorted_profiler_args = { + arg.dest: arg + for config in get_sorted_profilers(runtime) + for arg in get_profiler_arguments(runtime, config.profiler_name) + } + for arg in sorted_profiler_args.values(): + profiler_arg_kwargs = arg.get_dict() + name = profiler_arg_kwargs.pop("name") + arg_group.add_argument(name, **profiler_arg_kwargs) def verify_preconditions(args: configargparse.Namespace, processes_to_profile: Optional[List[Process]]) -> None: diff --git a/gprofiler/profilers/factory.py b/gprofiler/profilers/factory.py index b6d93e933..e592e3c89 100644 --- a/gprofiler/profilers/factory.py +++ b/gprofiler/profilers/factory.py @@ -2,7 +2,6 @@ from typing import TYPE_CHECKING, Any, List, Tuple, Union from gprofiler.log import get_logger_adapter -from gprofiler.metadata.system_metadata import get_arch from gprofiler.profilers.perf import SystemProfiler from gprofiler.profilers.profiler_base import NoopProfiler from gprofiler.profilers.registry import ( @@ -30,9 +29,9 @@ def get_profilers( if profiling_mode == "none": return system_profiler, process_profilers_instances - arch = get_arch() + for runtime in get_profilers_registry(): - runtime_args_prefix = runtime.lower().replace("-", "_") + runtime_args_prefix = runtime.lower() runtime_mode = user_args.get(f"{runtime_args_prefix}_mode") if runtime_mode in ProfilerConfig.DISABLED_MODES: continue @@ -44,15 +43,13 @@ def get_profilers( selected_configs: List[ProfilerConfig] = [] for config in requested_configs: profiler_name = config.profiler_name - if arch not in config.get_supported_archs() and len(requested_configs) == 1: - logger.warning(f"Disabling {profiler_name} because it doesn't support this architecture ({arch})") - continue if profiling_mode not in config.supported_profiling_modes: logger.warning( f"Disabling {profiler_name} because it doesn't support profiling mode {profiling_mode!r}" ) continue selected_configs.append(config) + if not selected_configs: logger.warning(f"Disabling {runtime} profiling because no profilers were selected") continue @@ -81,7 +78,7 @@ def get_profilers( logger.critical( f"Couldn't create the {profiler_name} profiler for runtime {runtime}, not continuing." f" Request different profiler for runtime with --{runtime_args_prefix}-mode, or disable" - f" {runtime} profiling with --no-{runtime_args_prefix} to disable this profiler", + f" {runtime} profiling with --{runtime_args_prefix}-mode=disabled to disable this profiler", exc_info=True, ) sys.exit(1) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 43573a86b..158818ba3 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -155,7 +155,6 @@ def make_application_metadata(self, process: Process) -> Dict[str, Any]: @register_profiler( "Python", profiler_name="PySpy", - # py-spy is like pyspy, it's confusing and I mix between them possible_modes=["auto", "pyspy", "py-spy"], default_mode="auto", # we build pyspy for both,. diff --git a/gprofiler/profilers/python_ebpf.py b/gprofiler/profilers/python_ebpf.py index 4954fe4a0..7d3dde55e 100644 --- a/gprofiler/profilers/python_ebpf.py +++ b/gprofiler/profilers/python_ebpf.py @@ -46,7 +46,6 @@ class PythonEbpfError(CalledProcessError): "Python", profiler_name="PyPerf", is_preferred=True, - # py-spy is like pyspy, it's confusing and I mix between them possible_modes=["auto", "pyperf"], default_mode="auto", # we build pyperf only for x86_64. diff --git a/gprofiler/profilers/registry.py b/gprofiler/profilers/registry.py index ebd50b178..b9817c82f 100644 --- a/gprofiler/profilers/registry.py +++ b/gprofiler/profilers/registry.py @@ -1,6 +1,6 @@ from collections import defaultdict from dataclasses import asdict, dataclass -from typing import Any, Callable, Dict, List, Optional, Sequence, Type, Union +from typing import Any, Callable, Dict, List, Optional, Sequence, Set, Type, Union from gprofiler.metadata.system_metadata import get_arch from gprofiler.platform import is_windows @@ -133,21 +133,25 @@ def get_profilers_by_name() -> Dict[str, ProfilerConfig]: def get_runtime_possible_modes(runtime: str) -> List[str]: - possible_modes: List[str] = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime]) > 1 else [] - for config in profilers_config[runtime]: - possible_modes += [m for m in config.get_active_modes() if m not in possible_modes] - possible_modes += ProfilerConfig.DISABLED_MODES - return possible_modes + """ + Get profiler modes supported for given runtime and available for current architecture. + """ + arch = get_arch() + added_modes: Set[str] = set() + for config in (c for c in profilers_config[runtime] if arch in c.get_supported_archs()): + added_modes.update(config.get_active_modes()) + initial_modes = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime]) > 1 else [] + return initial_modes + sorted(added_modes) + ProfilerConfig.DISABLED_MODES def get_sorted_profilers(runtime: str) -> List[ProfilerConfig]: """ - Get all profiler configs registered for given runtime sorted by preference. + Get all profiler configs registered for given runtime filtered for current architecture and sorted by preference. """ arch = get_arch() profiler_configs = sorted( - profilers_config[runtime], - key=lambda c: (arch in c.get_supported_archs(), c.is_preferred, c.profiler_name), + (c for c in profilers_config[runtime] if arch in c.get_supported_archs()), + key=lambda c: (c.is_preferred, c.profiler_name), reverse=True, ) return profiler_configs diff --git a/tests/test_python.py b/tests/test_python.py index e843052c1..fd14b8fdb 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -3,16 +3,18 @@ # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # import os -from contextlib import _GeneratorContextManager +from contextlib import contextmanager from pathlib import Path -from typing import Callable, List +from typing import List import psutil import pytest from docker import DockerClient +from docker.models.containers import Container from docker.models.images import Image from granulate_utils.linux.process import is_musl +from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.python import PySpyProfiler from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from tests.conftest import AssertInCollapsed @@ -20,6 +22,7 @@ assert_function_in_collapsed, is_aarch64, is_pattern_in_collapsed, + make_python_profiler, snapshot_pid_collapsed, snapshot_pid_profile, start_gprofiler_in_container_for_one_session, @@ -32,14 +35,22 @@ def runtime() -> str: return "python" +@contextmanager +def stop_container_after_use(container: Container) -> Container: + try: + yield container + finally: + container.stop() + + @pytest.mark.parametrize("in_container", [True]) @pytest.mark.parametrize("application_image_tag", ["libpython"]) @pytest.mark.parametrize("profiler_type", ["pyspy"]) def test_python_select_by_libpython( application_pid: int, assert_collapsed: AssertInCollapsed, - profiler_flags: List[str], - make_profiler_instance: Callable[[], _GeneratorContextManager], + profiler_state: ProfilerState, + profiler_type: str, ) -> None: """ Tests that profiling of processes running Python, whose basename(readlink("/proc/pid/exe")) isn't "python" @@ -47,10 +58,8 @@ def test_python_select_by_libpython( We expect to select these because they have "libpython" in their "/proc/pid/maps". This test runs a Python named "shmython". """ - profiler_flags.extend(["-f", "1000", "-d", "1"]) - with make_profiler_instance() as profiler: - with profiler: - process_collapsed = snapshot_pid_collapsed(profiler, application_pid) + with make_python_profiler(1000, 1, profiler_state, profiler_type, True, None, False) as profiler: + process_collapsed = snapshot_pid_collapsed(profiler, application_pid) assert_collapsed(process_collapsed) assert all(stack.startswith("shmython") for stack in process_collapsed.keys()) @@ -87,8 +96,7 @@ def test_python_matrix( assert_collapsed: AssertInCollapsed, profiler_type: str, application_image_tag: str, - profiler_flags: List[str], - make_profiler_instance: Callable[[], _GeneratorContextManager], + profiler_state: ProfilerState, ) -> None: python_version, libc, app = application_image_tag.split("-") @@ -110,10 +118,8 @@ def test_python_matrix( if python_version in ["3.7", "3.8", "3.9", "3.10", "3.11"] and profiler_type == "py-spy" and libc == "musl": pytest.xfail("This combination fails, see https://github.com/Granulate/gprofiler/issues/714") - profiler_flags.extend(["-f", "1000", "-d", "2"]) - with make_profiler_instance() as profiler: - with profiler: - profile = snapshot_pid_profile(profiler, application_pid) + with make_python_profiler(1000, 2, profiler_state, profiler_type, True, None, False) as profiler: + profile = snapshot_pid_profile(profiler, application_pid) collapsed = profile.stacks @@ -162,18 +168,15 @@ def test_dso_name_in_pyperf_profile( profiler_type: str, application_image_tag: str, insert_dso_name: bool, - profiler_flags: List[str], - make_profiler_instance: Callable[[], _GeneratorContextManager], + profiler_state: ProfilerState, ) -> None: if is_aarch64() and profiler_type == "pyperf": pytest.skip( "PyPerf doesn't support aarch64 architecture, see https://github.com/Granulate/gprofiler/issues/499" ) - profiler_flags.extend(["-f", "1000", "-d", "2"]) - with make_profiler_instance() as profiler: - with profiler: - profile = snapshot_pid_profile(profiler, application_pid) + with make_python_profiler(1000, 2, profiler_state, profiler_type, True, None, False) as profiler: + profile = snapshot_pid_profile(profiler, application_pid) python_version, _, _ = application_image_tag.split("-") interpreter_frame = "PyEval_EvalFrameEx" if python_version == "2.7" else "_PyEval_EvalFrameDefault" collapsed = profile.stacks @@ -212,9 +215,15 @@ def test_select_specific_python_profiler( # make sure the default behavior, with implicit enabled mode leads to auto selection profiler_flags.remove(f"--python-mode={profiler_type}") profiler_flags.extend(["--no-perf"]) - gprofiler = start_gprofiler_in_container_for_one_session( - docker_client, gprofiler_docker_image, output_directory, output_collapsed, runtime_specific_args, profiler_flags - ) - wait_for_log(gprofiler, "gProfiler initialized and ready to start profiling", 0, timeout=7) - assert f"Initialized {profiler_class_name}".encode() in gprofiler.logs() - gprofiler.remove(force=True) + with stop_container_after_use( + start_gprofiler_in_container_for_one_session( + docker_client, + gprofiler_docker_image, + output_directory, + output_collapsed, + runtime_specific_args, + profiler_flags, + ) + ) as gprofiler: + wait_for_log(gprofiler, "gProfiler initialized and ready to start profiling", 0, timeout=7) + assert f"Initialized {profiler_class_name}".encode() in gprofiler.logs() diff --git a/tests/utils.py b/tests/utils.py index 281242f98..6330794fe 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -29,7 +29,9 @@ JavaFlagCollectionOptions, JavaProfiler, ) -from gprofiler.profilers.profiler_base import ProfilerInterface +from gprofiler.profilers.profiler_base import ProfilerBase, ProfilerInterface +from gprofiler.profilers.python import PySpyProfiler +from gprofiler.profilers.python_ebpf import PythonEbpfProfiler from gprofiler.utils import remove_path, wait_event RUNTIME_PROFILERS = [ @@ -235,6 +237,32 @@ def make_java_profiler( ) +def make_python_profiler( + frequency: int, + duration: int, + profiler_state: ProfilerState, + python_mode: str, + python_add_versions: bool, + python_pyperf_user_stacks_pages: Optional[int], + python_pyperf_verbose: bool, +) -> ProfilerBase: + assert python_mode in ["pyperf", "pyspy"] + if python_mode == "pyperf": + return PythonEbpfProfiler( + frequency, + duration, + profiler_state, + python_mode=python_mode, + python_add_versions=python_add_versions, + python_pyperf_user_stacks_pages=python_pyperf_user_stacks_pages, + python_pyperf_verbose=python_pyperf_verbose, + ) + else: + return PySpyProfiler( + frequency, duration, profiler_state, python_mode=python_mode, python_add_versions=python_add_versions + ) + + def start_gprofiler_in_container_for_one_session( docker_client: DockerClient, gprofiler_docker_image: Image, From 05361129c0abb360ac144e46b94c87dcfe229e2f Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Wed, 12 Jul 2023 11:07:35 +0200 Subject: [PATCH 06/11] Show options only for profilers available on current architecture --- gprofiler/main.py | 16 ++++++++++++++-- gprofiler/profilers/registry.py | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gprofiler/main.py b/gprofiler/main.py index 9c2f08b20..bb900e462 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -806,7 +806,7 @@ def parse_cmd_args() -> configargparse.Namespace: if not args.upload_results and not args.output_dir: parser.error("Must pass at least one output method (--upload-results / --output-dir)") - if args.perf_dwarf_stack_size > 65528: + if args.perf_mode not in ("disabled", "none") and args.perf_dwarf_stack_size > 65528: parser.error("--perf-dwarf-stack-size maximum size is 65528") if args.profiling_mode == CPU_PROFILING_MODE and args.perf_mode in ("dwarf", "smart") and args.frequency > 100: @@ -825,8 +825,20 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: # add command-line arguments for each profiling runtime, but only for profilers that are working # with current architecture. for runtime in get_profilers_registry(): + runtime_possible_modes = get_runtime_possible_modes(runtime) arg_group = parser.add_argument_group(runtime) mode_var = f"{runtime.lower()}_mode" + if not runtime_possible_modes: + # if no mode is possible for this runtime, skip this runtime, and register it as disabled + # to overcome issue with Perf showing up on Windows + parser.add_argument( + f"--{runtime.lower()}-mode", + dest=mode_var, + default="disabled", + help=configargparse.SUPPRESS, + ) + continue + # TODO: organize options and usage for runtime - single source of runtime options? preferred_profiler = get_preferred_or_first_profiler(runtime) arg_group.add_argument( @@ -834,7 +846,7 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: dest=mode_var, default=preferred_profiler.default_mode, help=preferred_profiler.profiler_mode_help, - choices=get_runtime_possible_modes(runtime), + choices=runtime_possible_modes, ) arg_group.add_argument( f"--no-{runtime.lower()}", diff --git a/gprofiler/profilers/registry.py b/gprofiler/profilers/registry.py index b9817c82f..96de1a830 100644 --- a/gprofiler/profilers/registry.py +++ b/gprofiler/profilers/registry.py @@ -140,6 +140,8 @@ def get_runtime_possible_modes(runtime: str) -> List[str]: added_modes: Set[str] = set() for config in (c for c in profilers_config[runtime] if arch in c.get_supported_archs()): added_modes.update(config.get_active_modes()) + if not added_modes: + return [] initial_modes = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime]) > 1 else [] return initial_modes + sorted(added_modes) + ProfilerConfig.DISABLED_MODES From 918f4e61ab1c6a350efcb9234c721a31402abc2b Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Wed, 12 Jul 2023 13:31:57 +0200 Subject: [PATCH 07/11] Fix tests --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 6330794fe..492e2b1f1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -246,7 +246,7 @@ def make_python_profiler( python_pyperf_user_stacks_pages: Optional[int], python_pyperf_verbose: bool, ) -> ProfilerBase: - assert python_mode in ["pyperf", "pyspy"] + assert python_mode in ["pyperf", "pyspy", "py-spy"] if python_mode == "pyperf": return PythonEbpfProfiler( frequency, From b3c7e0ca72d7c88e0e2b7332e63ca2ef515ef663 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Thu, 13 Jul 2023 10:46:05 +0200 Subject: [PATCH 08/11] Introduce ProfilingRuntime --- gprofiler/main.py | 39 ++++---- gprofiler/profilers/dotnet.py | 9 +- gprofiler/profilers/factory.py | 24 ++--- gprofiler/profilers/java.py | 12 ++- gprofiler/profilers/perf.py | 44 ++++++--- gprofiler/profilers/php.py | 9 +- gprofiler/profilers/python.py | 32 +++++- gprofiler/profilers/python_ebpf.py | 20 +--- gprofiler/profilers/registry.py | 150 +++++++++++++++++------------ gprofiler/profilers/ruby.py | 9 +- tests/conftest.py | 36 +------ tests/test_runtime_profilers.py | 97 ++++++++++++++----- 12 files changed, 281 insertions(+), 200 deletions(-) diff --git a/gprofiler/main.py b/gprofiler/main.py index bb900e462..066138b45 100644 --- a/gprofiler/main.py +++ b/gprofiler/main.py @@ -50,10 +50,9 @@ from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.profiler_base import NoopProfiler, ProcessProfilerBase, ProfilerInterface from gprofiler.profilers.registry import ( - get_preferred_or_first_profiler, - get_profiler_arguments, - get_profilers_registry, + ProfilerArgument, get_runtime_possible_modes, + get_runtimes_registry, get_sorted_profilers, ) from gprofiler.spark.sampler import SparkSampler @@ -824,8 +823,10 @@ def parse_cmd_args() -> configargparse.Namespace: def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: # add command-line arguments for each profiling runtime, but only for profilers that are working # with current architecture. - for runtime in get_profilers_registry(): - runtime_possible_modes = get_runtime_possible_modes(runtime) + runtimes_registry = get_runtimes_registry() + for runtime_class, runtime_config in runtimes_registry.items(): + runtime = runtime_config.runtime_name + runtime_possible_modes = get_runtime_possible_modes(runtime_class) arg_group = parser.add_argument_group(runtime) mode_var = f"{runtime.lower()}_mode" if not runtime_possible_modes: @@ -839,13 +840,11 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: ) continue - # TODO: organize options and usage for runtime - single source of runtime options? - preferred_profiler = get_preferred_or_first_profiler(runtime) arg_group.add_argument( f"--{runtime.lower()}-mode", dest=mode_var, - default=preferred_profiler.default_mode, - help=preferred_profiler.profiler_mode_help, + default=runtime_config.default_mode, + help=runtime_config.mode_help, choices=runtime_possible_modes, ) arg_group.add_argument( @@ -854,18 +853,20 @@ def _add_profilers_arguments(parser: configargparse.ArgumentParser) -> None: const="disabled", dest=mode_var, default=True, - help=preferred_profiler.disablement_help, + help=runtime_config.disablement_help, ) - # select arguments from sorted filtered list of profilers in current runtime - sorted_profiler_args = { - arg.dest: arg - for config in get_sorted_profilers(runtime) - for arg in get_profiler_arguments(runtime, config.profiler_name) - } - for arg in sorted_profiler_args.values(): + # add each available profiler's arguments and runtime common arguments + profiling_args: List[ProfilerArgument] = [] + profiling_args.extend(runtime_config.common_arguments) + for config in get_sorted_profilers(runtime_class): + profiling_args.extend(config.profiler_args) + + for arg in profiling_args: profiler_arg_kwargs = arg.get_dict() - name = profiler_arg_kwargs.pop("name") - arg_group.add_argument(name, **profiler_arg_kwargs) + # do not add parser entries for profiler internal arguments + if "internal" not in profiler_arg_kwargs: + name = profiler_arg_kwargs.pop("name") + arg_group.add_argument(name, **profiler_arg_kwargs) def verify_preconditions(args: configargparse.Namespace, processes_to_profile: Optional[List[Process]]) -> None: diff --git a/gprofiler/profilers/dotnet.py b/gprofiler/profilers/dotnet.py index f52a48a47..3e03e1d5a 100644 --- a/gprofiler/profilers/dotnet.py +++ b/gprofiler/profilers/dotnet.py @@ -19,7 +19,7 @@ from gprofiler.platform import is_windows from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.profiler_base import ProcessProfilerBase -from gprofiler.profilers.registry import register_profiler +from gprofiler.profilers.registry import ProfilingRuntime, register_profiler, register_runtime from gprofiler.utils import pgrep_exe, pgrep_maps, random_prefix, removed_path, resource_path, run_process from gprofiler.utils.process import process_comm from gprofiler.utils.speedscope import load_speedscope_as_collapsed @@ -49,12 +49,17 @@ def make_application_metadata(self, process: Process) -> Dict[str, Any]: return metadata +@register_runtime("dotnet", default_mode="disabled") +class DotnetRuntime(ProfilingRuntime): + pass + + @register_profiler( "dotnet", + runtime_class=DotnetRuntime, possible_modes=["dotnet-trace", "disabled"], supported_archs=["x86_64", "aarch64"], supported_windows_archs=["AMD64"], - default_mode="disabled", supported_profiling_modes=["cpu"], ) class DotnetProfiler(ProcessProfilerBase): diff --git a/gprofiler/profilers/factory.py b/gprofiler/profilers/factory.py index e592e3c89..662638033 100644 --- a/gprofiler/profilers/factory.py +++ b/gprofiler/profilers/factory.py @@ -4,12 +4,7 @@ from gprofiler.log import get_logger_adapter from gprofiler.profilers.perf import SystemProfiler from gprofiler.profilers.profiler_base import NoopProfiler -from gprofiler.profilers.registry import ( - ProfilerConfig, - get_profiler_arguments, - get_profilers_registry, - get_sorted_profilers, -) +from gprofiler.profilers.registry import ProfilerConfig, get_runtimes_registry, get_sorted_profilers if TYPE_CHECKING: from gprofiler.gprofiler_types import UserArgs @@ -30,13 +25,14 @@ def get_profilers( if profiling_mode == "none": return system_profiler, process_profilers_instances - for runtime in get_profilers_registry(): + for runtime_class, runtime_config in get_runtimes_registry().items(): + runtime = runtime_config.runtime_name runtime_args_prefix = runtime.lower() runtime_mode = user_args.get(f"{runtime_args_prefix}_mode") if runtime_mode in ProfilerConfig.DISABLED_MODES: continue # select configs supporting requested runtime_mode or all configs in order of preference - requested_configs: List[ProfilerConfig] = get_sorted_profilers(runtime) + requested_configs: List[ProfilerConfig] = get_sorted_profilers(runtime_class) if runtime_mode != ProfilerConfig.ENABLED_MODE: requested_configs = [c for c in requested_configs if runtime_mode in c.get_active_modes()] # select profilers that support this architecture and profiling mode @@ -55,18 +51,14 @@ def get_profilers( continue # create instances of selected profilers one by one, select first that is ready ready_profiler = None - runtime_arg_names = [arg.dest for config in get_profilers_registry()[runtime] for arg in config.profiler_args] + mode_var = f"{runtime.lower()}_mode" + runtime_arg_names: List[str] = [arg.dest for arg in runtime_config.common_arguments] + [mode_var] for profiler_config in selected_configs: profiler_name = profiler_config.profiler_name profiler_kwargs = profiler_init_kwargs.copy() - profiler_arg_names = [arg.dest for arg in get_profiler_arguments(runtime, profiler_name)] + profiler_arg_names = [arg.dest for arg in profiler_config.profiler_args] for key, value in user_args.items(): - if ( - key in profiler_arg_names - or key in COMMON_PROFILER_ARGUMENT_NAMES - or key.startswith(runtime_args_prefix) - and key not in runtime_arg_names - ): + if key in profiler_arg_names or key in runtime_arg_names or key in COMMON_PROFILER_ARGUMENT_NAMES: profiler_kwargs[key] = value try: profiler_instance = profiler_config.profiler_class(**profiler_kwargs) diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 9894bec34..ef6e1f0cf 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -67,7 +67,7 @@ from gprofiler.metadata.application_metadata import ApplicationMetadata from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.profiler_base import SpawningProcessProfilerBase -from gprofiler.profilers.registry import ProfilerArgument, register_profiler +from gprofiler.profilers.registry import ProfilerArgument, ProfilingRuntime, register_profiler, register_runtime from gprofiler.utils import ( GPROFILER_DIRECTORY_NAME, TEMPORARY_STORAGE_PATH, @@ -738,10 +738,18 @@ def read_output(self) -> Optional[str]: raise +@register_runtime( + "Java", + default_mode="ap", +) +class JavaRuntime(ProfilingRuntime): + pass + + @register_profiler( "Java", + runtime_class=JavaRuntime, possible_modes=["ap", "disabled"], - default_mode="ap", supported_archs=["x86_64", "aarch64"], profiler_arguments=[ ProfilerArgument( diff --git a/gprofiler/profilers/perf.py b/gprofiler/profilers/perf.py index 1db325c8e..821d1f86e 100644 --- a/gprofiler/profilers/perf.py +++ b/gprofiler/profilers/perf.py @@ -34,7 +34,13 @@ from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.node import clean_up_node_maps, generate_map_for_node_processes, get_node_processes from gprofiler.profilers.profiler_base import ProfilerBase -from gprofiler.profilers.registry import ProfilerArgument, register_profiler +from gprofiler.profilers.registry import ( + InternalArgument, + ProfilerArgument, + ProfilingRuntime, + register_profiler, + register_runtime, +) from gprofiler.utils import ( reap_process, remove_files_by_prefix, @@ -373,15 +379,33 @@ def wait_and_script(self) -> str: remove_path(inject_data, missing_ok=True) -@register_profiler( +@register_runtime( "Perf", - possible_modes=["fp", "dwarf", "smart", "disabled"], default_mode="smart", - supported_archs=["x86_64", "aarch64"], - profiler_mode_argument_help="Run perf with either FP (Frame Pointers), DWARF, or run both and intelligently merge" + mode_help="Run perf with either FP (Frame Pointers), DWARF, or run both and intelligently merge" " them by choosing the best result per process. If 'disabled' is chosen, do not invoke" " 'perf' at all. The output, in that case, is the concatenation of the results from all" " of the runtime profilers. Defaults to 'smart'.", + common_arguments=[ + ProfilerArgument( + "--perf-no-memory-restart", + help="Disable checking if perf used memory exceeds threshold and restarting perf", + action="store_false", + dest="perf_memory_restart", + ), + ], + disablement_help="Disable the global perf of processes," + " and instead only concatenate runtime-specific profilers results", +) +class PerfRuntime(ProfilingRuntime): + pass + + +@register_profiler( + "Perf", + runtime_class=PerfRuntime, + possible_modes=["fp", "dwarf", "smart", "disabled"], + supported_archs=["x86_64", "aarch64"], profiler_arguments=[ ProfilerArgument( "--perf-dwarf-stack-size", @@ -391,15 +415,9 @@ def wait_and_script(self) -> str: default=DEFAULT_PERF_DWARF_STACK_SIZE, dest="perf_dwarf_stack_size", ), - ProfilerArgument( - "--perf-no-memory-restart", - help="Disable checking if perf used memory exceeds threshold and restarting perf", - action="store_false", - dest="perf_memory_restart", - ), + InternalArgument(dest="perf_inject"), + InternalArgument(dest="perf_node_attach"), ], - disablement_help="Disable the global perf of processes," - " and instead only concatenate runtime-specific profilers results", supported_profiling_modes=["cpu"], ) class SystemProfiler(ProfilerBase): diff --git a/gprofiler/profilers/php.py b/gprofiler/profilers/php.py index 739fbb16f..e49cfa7c7 100644 --- a/gprofiler/profilers/php.py +++ b/gprofiler/profilers/php.py @@ -18,7 +18,7 @@ from gprofiler.log import get_logger_adapter from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.profiler_base import ProfilerBase -from gprofiler.profilers.registry import ProfilerArgument, register_profiler +from gprofiler.profilers.registry import ProfilerArgument, ProfilingRuntime, register_profiler, register_runtime from gprofiler.utils import random_prefix, reap_process, resource_path, start_process, wait_event logger = get_logger_adapter(__name__) @@ -26,11 +26,16 @@ DEFAULT_PROCESS_FILTER = "php-fpm" +@register_runtime("PHP", default_mode="disabled") +class PHPRuntime(ProfilingRuntime): + pass + + @register_profiler( "PHP", + runtime_class=PHPRuntime, possible_modes=["phpspy", "disabled"], supported_archs=["x86_64", "aarch64"], - default_mode="disabled", profiler_arguments=[ ProfilerArgument( "--php-proc-filter", diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 158818ba3..d870b8a04 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -35,7 +35,7 @@ from gprofiler.platform import is_linux, is_windows from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.profiler_base import SpawningProcessProfilerBase -from gprofiler.profilers.registry import register_profiler +from gprofiler.profilers.registry import ProfilerArgument, ProfilingRuntime, register_profiler, register_runtime from gprofiler.utils import pgrep_exe, pgrep_maps, random_prefix, removed_path, resource_path, run_process from gprofiler.utils.collapsed_format import parse_one_collapsed_file from gprofiler.utils.process import process_comm, search_proc_maps @@ -152,15 +152,37 @@ def make_application_metadata(self, process: Process) -> Dict[str, Any]: return metadata -@register_profiler( +@register_runtime( "Python", - profiler_name="PySpy", - possible_modes=["auto", "pyspy", "py-spy"], default_mode="auto", + mode_help="Select the Python profiling mode: auto (try PyPerf, resort to py-spy if it fails), " + "pyspy (always use py-spy), pyperf (always use PyPerf, and avoid py-spy even if it fails)" + " or disabled (no runtime profilers for Python).", + common_arguments=[ + # TODO should be prefixed with --python- + ProfilerArgument( + "--no-python-versions", + dest="python_add_versions", + action="store_false", + default=True, + help="Don't add version information to Python frames. If not set, frames from packages are displayed with " + "the name of the package and its version, and frames from Python built-in modules are displayed with " + "Python's full version.", + ), + ], +) +class PythonRuntime(ProfilingRuntime): + pass + + +@register_profiler( + "PySpy", + runtime_class=PythonRuntime, + possible_modes=["auto", "pyspy", "py-spy"], # we build pyspy for both,. supported_archs=["x86_64", "aarch64"], supported_windows_archs=["AMD64"], - # profiler arguments are defined by preferred profiler of the runtime, that is PythonEbpfProfiler + # no specific arguments for PySpy besides the common args from runtime profiler_arguments=[], supported_profiling_modes=["cpu"], ) diff --git a/gprofiler/profilers/python_ebpf.py b/gprofiler/profilers/python_ebpf.py index 7d3dde55e..7d1070e7f 100644 --- a/gprofiler/profilers/python_ebpf.py +++ b/gprofiler/profilers/python_ebpf.py @@ -20,6 +20,7 @@ from gprofiler.profiler_state import ProfilerState from gprofiler.profilers import python from gprofiler.profilers.profiler_base import ProfilerBase +from gprofiler.profilers.python import PythonRuntime from gprofiler.profilers.registry import ProfilerArgument, register_profiler from gprofiler.utils import ( poll_process, @@ -43,27 +44,12 @@ class PythonEbpfError(CalledProcessError): @register_profiler( - "Python", - profiler_name="PyPerf", + "PyPerf", + runtime_class=PythonRuntime, is_preferred=True, possible_modes=["auto", "pyperf"], - default_mode="auto", - # we build pyperf only for x86_64. supported_archs=["x86_64"], - profiler_mode_argument_help="Select the Python profiling mode: auto (try PyPerf, resort to py-spy if it fails), " - "pyspy (always use py-spy), pyperf (always use PyPerf, and avoid py-spy even if it fails)" - " or disabled (no runtime profilers for Python).", profiler_arguments=[ - # TODO should be prefixed with --python- - ProfilerArgument( - "--no-python-versions", - dest="python_add_versions", - action="store_false", - default=True, - help="Don't add version information to Python frames. If not set, frames from packages are displayed with " - "the name of the package and its version, and frames from Python built-in modules are displayed with " - "Python's full version.", - ), # TODO should be prefixed with --python- ProfilerArgument( "--pyperf-user-stacks-pages", diff --git a/gprofiler/profilers/registry.py b/gprofiler/profilers/registry.py index 96de1a830..248d28af3 100644 --- a/gprofiler/profilers/registry.py +++ b/gprofiler/profilers/registry.py @@ -23,32 +23,56 @@ def get_dict(self) -> Dict[str, Any]: return {key: value for key, value in asdict(self).items() if value is not None} +@dataclass +class InternalArgument(ProfilerArgument): + """ + Represents arguments internal to profiler, provided only by initialization code not on command line. + Name should be empty and only dest field is meaningful, as it names the argument expected by profiler instance. + """ + + name: str = "" + dest: str = "" + internal: Optional[bool] = True + + def __post_init__(self) -> None: + assert ( + set(self.get_dict()) == {"dest", "name", "internal"} and self.name == "" + ), "InternalArgument doesn't use any other fields than dest" + + +class ProfilingRuntime: + pass + + +@dataclass +class RuntimeConfig: + runtime_name: str + default_mode: str + mode_help: Optional[str] + disablement_help: Optional[str] + common_arguments: List[ProfilerArgument] + + class ProfilerConfig: def __init__( self, profiler_name: str, - runtime: str, + runtime_class: Type[ProfilingRuntime], is_preferred: bool, - profiler_mode_help: Optional[str], - disablement_help: Optional[str], profiler_class: Any, possible_modes: List[str], supported_archs: List[str], supported_profiling_modes: List[str], supported_windows_archs: List[str] = None, - default_mode: str = "enabled", arguments: List[ProfilerArgument] = None, ) -> None: self.profiler_name = profiler_name - self.runtime = runtime + self.runtime_class = runtime_class self.is_preferred = is_preferred - self.profiler_mode_help = profiler_mode_help self.possible_modes = possible_modes self.supported_archs = supported_archs self.supported_windows_archs = supported_windows_archs if supported_windows_archs is not None else [] - self.default_mode = default_mode self.profiler_args: List[ProfilerArgument] = arguments if arguments is not None else [] - self.disablement_help = disablement_help self.profiler_class = profiler_class self.supported_profiling_modes = supported_profiling_modes @@ -66,55 +90,78 @@ def get_supported_archs(self) -> List[str]: return self.supported_windows_archs if is_windows() else self.supported_archs -profilers_config: Dict[str, List[ProfilerConfig]] = defaultdict(list) +runtimes_config: Dict[Type[ProfilingRuntime], RuntimeConfig] = {} +profilers_config: Dict[Type[ProfilingRuntime], List[ProfilerConfig]] = defaultdict(list) + + +def register_runtime( + runtime_name: str, + default_mode: str = "enabled", + mode_help: Optional[str] = None, + disablement_help: Optional[str] = None, + common_arguments: List[ProfilerArgument] = None, +) -> Any: + if mode_help is None: + mode_help = ( + f"Choose the mode for profiling {runtime_name} processes. '{default_mode}'" + f" to profile them with the default method, or 'disabled' to disable {runtime_name}-specific profiling" + ) + if disablement_help is None: + disablement_help = f"Disable the runtime-profiling of {runtime_name} processes" + + def runtime_decorator(runtime_class: Type[ProfilingRuntime]) -> Any: + assert runtime_class not in runtimes_config, f"Runtime {runtime_name} is already registered" + assert all( + arg.dest.startswith(runtime_name.lower()) for arg in common_arguments or [] + ), f"{runtime_name}: Runtime common args dest must be prefixed with the runtime name" + + runtimes_config[runtime_class] = RuntimeConfig( + runtime_name, + default_mode, + mode_help, + disablement_help, + common_arguments if common_arguments is not None else [], + ) + return runtime_class + + return runtime_decorator def register_profiler( - runtime: str, - default_mode: str, + profiler_name: str, + runtime_class: Type[ProfilingRuntime], possible_modes: List[str], supported_archs: List[str], supported_profiling_modes: List[str], - profiler_name: Optional[str] = None, is_preferred: bool = False, supported_windows_archs: Optional[List[str]] = None, - profiler_mode_argument_help: Optional[str] = None, profiler_arguments: Optional[List[ProfilerArgument]] = None, - disablement_help: Optional[str] = None, ) -> Any: - if profiler_name is None: - profiler_name = runtime - if profiler_mode_argument_help is None: - profiler_mode_argument_help = ( - f"Choose the mode for profiling {profiler_name} processes. '{default_mode}'" - f" to profile them with the default method, or 'disabled' to disable {profiler_name}-specific profiling" - ) # Add the legacy "none" value, which is replaced by "disabled" possible_modes.append("none") - if disablement_help is None: - disablement_help = f"Disable the runtime-profiling of {profiler_name} processes" def profiler_decorator(profiler_class: Any) -> Any: assert profiler_name is not None, "Profiler name must be defined" + assert ( + runtime_class in runtimes_config + ), f"Profiler {profiler_name} refers to runtime {runtime_class}, which is not registered." + runtime_name = runtimes_config[runtime_class].runtime_name assert profiler_name not in ( config.profiler_name for profilers in profilers_config.values() for config in profilers ), f"{profiler_name} is already registered!" assert all( - arg.dest.startswith(runtime.lower()) for arg in profiler_arguments or [] + arg.dest.startswith(runtime_name.lower()) for arg in profiler_arguments or [] ), f"{profiler_name}: Profiler args dest must be prefixed with the profiler runtime name" - profilers_config[runtime] += [ + profilers_config[runtime_class] += [ ProfilerConfig( profiler_name, - runtime, + runtime_class, is_preferred, - profiler_mode_argument_help, - disablement_help, profiler_class, possible_modes, supported_archs, supported_profiling_modes, supported_windows_archs, - default_mode, profiler_arguments, ) ] @@ -124,7 +171,11 @@ def profiler_decorator(profiler_class: Any) -> Any: return profiler_decorator -def get_profilers_registry() -> Dict[str, List[ProfilerConfig]]: +def get_runtimes_registry() -> Dict[Type[ProfilingRuntime], RuntimeConfig]: + return runtimes_config + + +def get_profilers_registry() -> Dict[Type[ProfilingRuntime], List[ProfilerConfig]]: return profilers_config @@ -132,57 +183,28 @@ def get_profilers_by_name() -> Dict[str, ProfilerConfig]: return {config.profiler_name: config for configs in profilers_config.values() for config in configs} -def get_runtime_possible_modes(runtime: str) -> List[str]: +def get_runtime_possible_modes(runtime_class: Type[ProfilingRuntime]) -> List[str]: """ Get profiler modes supported for given runtime and available for current architecture. """ arch = get_arch() added_modes: Set[str] = set() - for config in (c for c in profilers_config[runtime] if arch in c.get_supported_archs()): + for config in (c for c in profilers_config[runtime_class] if arch in c.get_supported_archs()): added_modes.update(config.get_active_modes()) if not added_modes: return [] - initial_modes = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime]) > 1 else [] + initial_modes = [ProfilerConfig.ENABLED_MODE] if len(profilers_config[runtime_class]) > 1 else [] return initial_modes + sorted(added_modes) + ProfilerConfig.DISABLED_MODES -def get_sorted_profilers(runtime: str) -> List[ProfilerConfig]: +def get_sorted_profilers(runtime_class: Type[ProfilingRuntime]) -> List[ProfilerConfig]: """ Get all profiler configs registered for given runtime filtered for current architecture and sorted by preference. """ arch = get_arch() profiler_configs = sorted( - (c for c in profilers_config[runtime] if arch in c.get_supported_archs()), + (c for c in profilers_config[runtime_class] if arch in c.get_supported_archs()), key=lambda c: (c.is_preferred, c.profiler_name), reverse=True, ) return profiler_configs - - -def get_preferred_or_first_profiler(runtime: str) -> ProfilerConfig: - return next(filter(lambda config: config.is_preferred, profilers_config[runtime]), profilers_config[runtime][0]) - - -def get_profiler_arguments(runtime: str, profiler_name: str) -> List[ProfilerArgument]: - """ - For now the common and specific profiler command-line arguments are defined together, at profiler - registration. - Once we implement a mechanism to hold runtime-wide options (including arguments), this function will be - obsolete. - """ - # Arguments can be distinguished by prefix of their dest variable name. - # Group all profiler arguments and exclude those that are prefixed with other profiler names. - runtime_lower = runtime.lower() - other_profiler_prefixes = [ - f"{runtime_lower}_{config.profiler_name.lower().replace('-', '_')}" - for config in profilers_config[runtime] - if config.profiler_name != profiler_name - ] - all_runtime_args: List[ProfilerArgument] = [ - arg - for config in profilers_config[runtime] - for arg in config.profiler_args - if arg.dest.startswith(runtime_lower) - ] - profiler_args = [arg for arg in all_runtime_args if False is any(map(arg.dest.startswith, other_profiler_prefixes))] - return profiler_args diff --git a/gprofiler/profilers/ruby.py b/gprofiler/profilers/ruby.py index ed4baf5f7..1befeeea0 100644 --- a/gprofiler/profilers/ruby.py +++ b/gprofiler/profilers/ruby.py @@ -20,7 +20,7 @@ from gprofiler.metadata.application_metadata import ApplicationMetadata from gprofiler.profiler_state import ProfilerState from gprofiler.profilers.profiler_base import SpawningProcessProfilerBase -from gprofiler.profilers.registry import register_profiler +from gprofiler.profilers.registry import ProfilingRuntime, register_profiler, register_runtime from gprofiler.utils import pgrep_maps, random_prefix, removed_path, resource_path, run_process from gprofiler.utils.collapsed_format import parse_one_collapsed_file from gprofiler.utils.process import process_comm, search_proc_maps @@ -54,11 +54,16 @@ def make_application_metadata(self, process: Process) -> Dict[str, Any]: return metadata +@register_runtime("Ruby", default_mode="rbspy") +class RubyRuntime(ProfilingRuntime): + pass + + @register_profiler( "Ruby", + runtime_class=RubyRuntime, possible_modes=["rbspy", "disabled"], supported_archs=["x86_64", "aarch64"], - default_mode="rbspy", supported_profiling_modes=["cpu"], ) class RbSpyProfiler(SpawningProcessProfilerBase): diff --git a/tests/conftest.py b/tests/conftest.py index f1a73361a..d3d79ee58 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,6 @@ import secrets import stat import subprocess -import sys from contextlib import _GeneratorContextManager, contextmanager from functools import lru_cache, partial from pathlib import Path @@ -20,7 +19,7 @@ from docker.models.containers import Container from docker.models.images import Image from psutil import Process -from pytest import FixtureRequest, MonkeyPatch, TempPathFactory, fixture +from pytest import FixtureRequest, TempPathFactory, fixture from gprofiler.consts import CPU_PROFILING_MODE from gprofiler.containers_client import ContainerNamesClient @@ -35,9 +34,7 @@ ) from gprofiler.metadata.enrichment import EnrichmentOptions from gprofiler.profiler_state import ProfilerState -from gprofiler.profilers.factory import get_profilers from gprofiler.profilers.java import AsyncProfiledProcess, JattachJcmdRunner -from gprofiler.profilers.profiler_base import ProfilerBase from gprofiler.state import init_state from tests import CONTAINERS_DIRECTORY, PHPSPY_DURATION from tests.utils import ( @@ -583,37 +580,6 @@ def profiler_flags(runtime: str, profiler_type: str) -> List[str]: return flags -@fixture -def make_profiler_instance( - profiler_flags: List[str], monkeypatch: MonkeyPatch, output_directory: Path, profiler_state: ProfilerState -) -> Callable[[], _GeneratorContextManager]: - """ - Deliver an instance of profiler selected by runtime and profiler_type. - """ - - # Reuse gProfiler flow to initialize specific profiler instance - one selected by runtime fixture. - @contextmanager - def _make_profiler_instance() -> Iterator[ProfilerBase]: - with monkeypatch.context() as m: - from gprofiler.main import parse_cmd_args - - # overide command-line args to enable only one process profiler - m.setattr( - sys, - "argv", - sys.argv[:1] + ["--output-dir", str(output_directory), "--no-perf"] + profiler_flags, - ) - # invoke gProfiler command-line parsing to collect arguments - args = parse_cmd_args() - # call factory to deliver initialized profiler instance - _, process_profilers = get_profilers(args.__dict__, profiler_state=profiler_state) - assert len(process_profilers) == 1 - profiler = process_profilers[0] - yield cast(ProfilerBase, profiler) - - return _make_profiler_instance - - @fixture(autouse=True, scope="session") def _init_profiler() -> None: """ diff --git a/tests/test_runtime_profilers.py b/tests/test_runtime_profilers.py index c6dc9e319..89e3919fc 100644 --- a/tests/test_runtime_profilers.py +++ b/tests/test_runtime_profilers.py @@ -4,7 +4,7 @@ # import sys from collections import defaultdict -from typing import Any, Dict, Iterable, List, Optional, Set, cast +from typing import Any, Dict, Iterable, List, Optional, Set, Type, cast import pytest from pytest import MonkeyPatch @@ -13,26 +13,45 @@ from gprofiler.gprofiler_types import ProcessToProfileData from gprofiler.profilers import registry from gprofiler.profilers.factory import get_profilers +from gprofiler.profilers.perf import PerfRuntime from gprofiler.profilers.profiler_base import NoopProfiler, ProfilerBase -from gprofiler.profilers.registry import ProfilerArgument, ProfilerConfig, register_profiler +from gprofiler.profilers.registry import ( + ProfilerArgument, + ProfilerConfig, + ProfilingRuntime, + RuntimeConfig, + register_profiler, + register_runtime, +) + + +class MockRuntime(ProfilingRuntime): + pass + + +def _register_mock_runtime( + runtime_name: str, + default_mode: str = "enabled", + runtime_class: Any = MockRuntime, + common_arguments: Optional[List[ProfilerArgument]] = None, +) -> None: + register_runtime(runtime_name, default_mode, common_arguments=common_arguments)(runtime_class) def _register_mock_profiler( - runtime: str, profiler_name: str, + runtime_class: Any, profiler_class: Any = NoopProfiler, is_preferred: bool = False, - default_mode: str = "disabled", possible_modes: List[str] = ["disabled"], supported_archs: List[str] = ["x86_64"], supported_profiling_modes: List[str] = ["cpu"], profiler_arguments: Optional[List[ProfilerArgument]] = None, ) -> None: register_profiler( - runtime=runtime, profiler_name=profiler_name, + runtime_class=runtime_class, is_preferred=is_preferred, - default_mode=default_mode, possible_modes=possible_modes, supported_archs=supported_archs, supported_profiling_modes=supported_profiling_modes, @@ -40,10 +59,16 @@ def _register_mock_profiler( )(profiler_class) -def _copy_of_registry(keys: Iterable[str] = []) -> Dict[str, List[ProfilerConfig]]: +def _subset_of_profilers( + keys: Iterable[Type[ProfilingRuntime]] = [], +) -> Dict[Type[ProfilingRuntime], List[ProfilerConfig]]: return defaultdict(list, {k: v[:] for (k, v) in registry.profilers_config.items() if k in keys}) +def _subset_of_runtimes(keys: Iterable[Type[ProfilingRuntime]] = []) -> Dict[Type[ProfilingRuntime], RuntimeConfig]: + return {k: v for (k, v) in registry.runtimes_config.items() if k in keys} + + def test_profiler_names_are_unique(monkeypatch: MonkeyPatch) -> None: """ Make sure that registered profilers are checked for uniqueness. @@ -53,9 +78,16 @@ def test_profiler_names_are_unique(monkeypatch: MonkeyPatch) -> None: with monkeypatch.context() as m: # clear registry before registering mock profilers m.setattr(profilers.registry, "profilers_config", defaultdict(list)) - _register_mock_profiler("python", profiler_name="mock", possible_modes=["mock-profiler", "disabled"]) + m.setattr(profilers.registry, "runtimes_config", _subset_of_runtimes(keys=[PerfRuntime])) + _register_mock_runtime("mock", runtime_class=MockRuntime) + + _register_mock_profiler( + profiler_name="mock", runtime_class=MockRuntime, possible_modes=["mock-profiler", "disabled"] + ) with pytest.raises(AssertionError) as excinfo: - _register_mock_profiler("ruby", profiler_name="mock", possible_modes=["mock-spy", "disabled"]) + _register_mock_profiler( + profiler_name="mock", runtime_class=PerfRuntime, possible_modes=["mock-spy", "disabled"] + ) assert "mock is already registered" in str(excinfo.value) @@ -79,16 +111,21 @@ def test_union_of_runtime_profilers_modes( with monkeypatch.context() as m: # clear registry before registering mock profilers; # keep Perf profiler, as some of its arguments (perf_dwarf_stack_size) are checked in command line parsing - m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + m.setattr(profilers.registry, "profilers_config", _subset_of_profilers(keys=[PerfRuntime])) + m.setattr(profilers.registry, "runtimes_config", _subset_of_runtimes(keys=[PerfRuntime])) + _register_mock_runtime("mock", runtime_class=MockRuntime) _register_mock_profiler( - "mock", profiler_name="mock-profiler", + runtime_class=MockRuntime, profiler_class=MockProfiler, possible_modes=["mock-profiler", "disabled"], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) _register_mock_profiler( - "mock", profiler_name="mock-perf", profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"] + profiler_name="mock-perf", + runtime_class=MockRuntime, + profiler_class=MockPerf, + possible_modes=["mock-perf", "disabled"], ) from gprofiler.main import parse_cmd_args @@ -115,17 +152,22 @@ def test_select_specific_runtime_profiler( ) -> None: with monkeypatch.context() as m: # clear registry before registering mock profilers - m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + m.setattr(profilers.registry, "profilers_config", _subset_of_profilers(keys=[PerfRuntime])) + m.setattr(profilers.registry, "runtimes_config", _subset_of_runtimes(keys=[PerfRuntime])) + _register_mock_runtime("mock", runtime_class=MockRuntime) _register_mock_profiler( - "mock", profiler_name="mock-profiler", + runtime_class=MockRuntime, profiler_class=MockProfiler, possible_modes=["mock-profiler", "disabled"], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) _register_mock_profiler( - "mock", profiler_name="mock-perf", profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"] + profiler_name="mock-perf", + runtime_class=MockRuntime, + profiler_class=MockPerf, + possible_modes=["mock-perf", "disabled"], ) from gprofiler.main import parse_cmd_args @@ -153,10 +195,12 @@ def test_auto_select_preferred_profiler( """ with monkeypatch.context() as m: # clear registry before registering mock profilers - m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) + m.setattr(profilers.registry, "profilers_config", _subset_of_profilers(keys=[PerfRuntime])) + m.setattr(profilers.registry, "runtimes_config", _subset_of_runtimes(keys=[PerfRuntime])) + _register_mock_runtime("mock", runtime_class=MockRuntime) _register_mock_profiler( - "mock", profiler_name="mock-profiler", + runtime_class=MockRuntime, profiler_class=MockProfiler, is_preferred="mock-profiler" == preferred_profiler, possible_modes=["mock-profiler", "disabled"], @@ -164,8 +208,8 @@ def test_auto_select_preferred_profiler( MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) _register_mock_profiler( - "mock", profiler_name="mock-perf", + runtime_class=MockRuntime, profiler_class=MockPerf, is_preferred="mock-perf" == preferred_profiler, possible_modes=["mock-perf", "disabled"], @@ -216,28 +260,35 @@ def test_assign_correct_profiler_arguments( """ with monkeypatch.context() as m: # clear registry before registering mock profilers - m.setattr(profilers.registry, "profilers_config", _copy_of_registry(keys=["Perf"])) - _register_mock_profiler( + m.setattr(profilers.registry, "profilers_config", _subset_of_profilers(keys=[PerfRuntime])) + m.setattr(profilers.registry, "runtimes_config", _subset_of_runtimes(keys=[PerfRuntime])) + _register_mock_runtime( "mock", + runtime_class=MockRuntime, + common_arguments=[ + ProfilerArgument("--mock-common-one", "mock_one"), + ProfilerArgument("--mock-common-two", "mock_two"), + ], + ) + _register_mock_profiler( profiler_name="mock-profiler", + runtime_class=MockRuntime, profiler_class=MockProfiler, is_preferred="mock-profiler" == preferred_profiler, possible_modes=["mock-profiler", "disabled"], profiler_arguments=[ - ProfilerArgument("--mock-common-one", "mock_one"), ProfilerArgument("--mock-mock-profiler-one", "mock_mock_profiler_one"), ], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) _register_mock_profiler( - "mock", profiler_name="mock-perf", + runtime_class=MockRuntime, profiler_class=MockPerf, is_preferred="mock-perf" == preferred_profiler, possible_modes=["mock-perf", "disabled"], profiler_arguments=[ - ProfilerArgument("--mock-common-two", "mock_two"), ProfilerArgument("--mock-mock-perf-two", "mock_mock_perf_two"), ], ) From 3a7b9af7cc710a4ae1f5288cfaee92de35a0ef76 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Thu, 13 Jul 2023 14:31:21 +0200 Subject: [PATCH 09/11] Fix tests on aarch --- tests/test_python.py | 4 ++-- tests/test_runtime_profilers.py | 7 +++++++ tests/utils.py | 6 +++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/test_python.py b/tests/test_python.py index fd14b8fdb..fbd461c3c 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -192,8 +192,8 @@ def test_dso_name_in_pyperf_profile( [ ("python", "py-spy", PySpyProfiler.__name__), ("python", "pyperf", PythonEbpfProfiler.__name__), - ("python", "auto", PythonEbpfProfiler.__name__), - ("python", "enabled", PythonEbpfProfiler.__name__), + ("python", "auto", PythonEbpfProfiler.__name__ if not is_aarch64() else PySpyProfiler.__name__), + ("python", "enabled", PythonEbpfProfiler.__name__ if not is_aarch64() else PySpyProfiler.__name__), ], ) def test_select_specific_python_profiler( diff --git a/tests/test_runtime_profilers.py b/tests/test_runtime_profilers.py index 89e3919fc..36dc77122 100644 --- a/tests/test_runtime_profilers.py +++ b/tests/test_runtime_profilers.py @@ -23,6 +23,7 @@ register_profiler, register_runtime, ) +from tests.utils import get_arch class MockRuntime(ProfilingRuntime): @@ -160,6 +161,7 @@ def test_select_specific_runtime_profiler( runtime_class=MockRuntime, profiler_class=MockProfiler, possible_modes=["mock-profiler", "disabled"], + supported_archs=[get_arch()], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) @@ -168,6 +170,7 @@ def test_select_specific_runtime_profiler( runtime_class=MockRuntime, profiler_class=MockPerf, possible_modes=["mock-perf", "disabled"], + supported_archs=[get_arch()], ) from gprofiler.main import parse_cmd_args @@ -204,6 +207,7 @@ def test_auto_select_preferred_profiler( profiler_class=MockProfiler, is_preferred="mock-profiler" == preferred_profiler, possible_modes=["mock-profiler", "disabled"], + supported_archs=[get_arch()], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) @@ -213,6 +217,7 @@ def test_auto_select_preferred_profiler( profiler_class=MockPerf, is_preferred="mock-perf" == preferred_profiler, possible_modes=["mock-perf", "disabled"], + supported_archs=[get_arch()], ) from gprofiler.main import parse_cmd_args @@ -279,6 +284,7 @@ def test_assign_correct_profiler_arguments( profiler_arguments=[ ProfilerArgument("--mock-mock-profiler-one", "mock_mock_profiler_one"), ], + supported_archs=[get_arch()], ) MockPerf = type("MockPerf", (MockProfiler,), dict(**MockProfiler.__dict__)) @@ -291,6 +297,7 @@ def test_assign_correct_profiler_arguments( profiler_arguments=[ ProfilerArgument("--mock-mock-perf-two", "mock_mock_perf_two"), ], + supported_archs=[get_arch()], ) from gprofiler.main import parse_cmd_args diff --git a/tests/utils.py b/tests/utils.py index 492e2b1f1..1d1665013 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -172,8 +172,12 @@ def is_pattern_in_collapsed(pattern: str, collapsed: StackToSampleCount) -> bool return any(regex.search(record) is not None for record in collapsed.keys()) +def get_arch() -> str: + return platform.machine() + + def is_aarch64() -> bool: - return platform.machine() == "aarch64" + return get_arch() == "aarch64" def assert_function_in_collapsed(function_name: str, collapsed: StackToSampleCount) -> None: From b41f5ec9801156b6a61a20c7ca68f3db34e9d94c Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Fri, 28 Jul 2023 12:14:12 +0200 Subject: [PATCH 10/11] Address comments in review --- gprofiler/profilers/python.py | 2 +- tests/conftest.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index d870b8a04..7612b2c04 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -179,7 +179,7 @@ class PythonRuntime(ProfilingRuntime): "PySpy", runtime_class=PythonRuntime, possible_modes=["auto", "pyspy", "py-spy"], - # we build pyspy for both,. + # we build pyspy for both supported_archs=["x86_64", "aarch64"], supported_windows_archs=["AMD64"], # no specific arguments for PySpy besides the common args from runtime diff --git a/tests/conftest.py b/tests/conftest.py index d3d79ee58..6f87331f0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -626,6 +626,7 @@ def python_version(in_container: bool, application_docker_container: Container) else: # If not running in a container the test application runs on host output = subprocess.check_output("python3 --version", stderr=subprocess.STDOUT, shell=True) + # Output is expected to look like e.g. "Python 3.9.7" return cast(str, output.decode().strip().split()[-1]) From 94a8010c62416f010d13912991f04b6e74ee64d5 Mon Sep 17 00:00:00 2001 From: Marcin Olszewski Date: Fri, 1 Sep 2023 13:54:20 +0200 Subject: [PATCH 11/11] Disable metadata collection to avoid slowdown of gProfiler startup --- tests/test_python.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_python.py b/tests/test_python.py index fbd461c3c..e9069bc02 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -214,7 +214,7 @@ def test_select_specific_python_profiler( elif profiler_type == "enabled": # make sure the default behavior, with implicit enabled mode leads to auto selection profiler_flags.remove(f"--python-mode={profiler_type}") - profiler_flags.extend(["--no-perf"]) + profiler_flags.extend(["--no-perf", "--disable-metadata-collection"]) with stop_container_after_use( start_gprofiler_in_container_for_one_session( docker_client, @@ -225,5 +225,5 @@ def test_select_specific_python_profiler( profiler_flags, ) ) as gprofiler: - wait_for_log(gprofiler, "gProfiler initialized and ready to start profiling", 0, timeout=7) + wait_for_log(gprofiler, "gProfiler initialized and ready to start profiling", 0, timeout=30) assert f"Initialized {profiler_class_name}".encode() in gprofiler.logs()