Decouple PySpyProfiler and EbpfProfiler from PythonProfiler

Author: marcin-ol

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),
         )

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

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)

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()