Skip to content

Commit 7fbbfde

Browse files
committed
gh-141592: final polish per review (docstring, unlink, with, comments)
Signed-off-by: Paresh Joshi <rahulj9223@gmail.com>
1 parent c56afcd commit 7fbbfde

1 file changed

Lines changed: 18 additions & 25 deletions

File tree

Lib/test/test_perf_profiler.py

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
assert_python_failure,
1212
assert_python_ok,
1313
)
14-
from test.support.os_helper import temp_dir
14+
from test.support.os_helper import temp_dir, unlink
1515

1616

1717
if not support.has_subprocess_support:
@@ -37,15 +37,8 @@ def supports_trampoline_profiling():
3737
class TestPerfTrampoline(unittest.TestCase):
3838

3939
def _cleanup_perf_map(self, pid):
40-
"""
41-
Helper to safely delete a specific perf map file.
42-
"""
43-
perf_map = pathlib.Path(f"/tmp/perf-{pid}.map")
44-
try:
45-
if perf_map.exists():
46-
perf_map.unlink()
47-
except OSError:
48-
pass
40+
"""Helper to safely delete a specific perf map file."""
41+
unlink(f"/tmp/perf-{pid}.map")
4942

5043
@unittest.skipIf(support.check_bolt_optimized(), "fails on BOLT instrumented binaries")
5144
def test_trampoline_works(self):
@@ -65,17 +58,14 @@ def baz():
6558
script = make_script(script_dir, "perftest", code)
6659
env = {**os.environ, "PYTHON_JIT": "0"}
6760

68-
process = subprocess.Popen(
61+
with subprocess.Popen(
6962
[sys.executable, "-Xperf", script],
7063
text=True,
7164
stderr=subprocess.PIPE,
7265
stdout=subprocess.PIPE,
7366
env=env,
74-
)
75-
76-
self.addCleanup(self._cleanup_perf_map, process.pid)
77-
78-
with process:
67+
) as process:
68+
self.addCleanup(self._cleanup_perf_map, process.pid)
7969
stdout, stderr = process.communicate()
8070

8171
self.assertEqual(stderr, "")
@@ -281,6 +271,8 @@ def perf_command_works():
281271
except (subprocess.SubprocessError, OSError):
282272
return False
283273

274+
# perf version does not return a version number on Fedora. Use presence
275+
# of "perf.data" in help as indicator that it's perf from Linux tools.
284276
if "perf.data" not in stdout:
285277
return False
286278

@@ -442,15 +434,8 @@ def baz(n):
442434
class TestPerfProfiler(unittest.TestCase, TestPerfProfilerMixin):
443435

444436
def _cleanup_perf_map(self, pid):
445-
"""
446-
Helper to safely delete a specific perf map file.
447-
"""
448-
perf_map = pathlib.Path(f"/tmp/perf-{pid}.map")
449-
try:
450-
if perf_map.exists():
451-
perf_map.unlink()
452-
except OSError:
453-
pass
437+
"""Helper to safely delete a specific perf map file."""
438+
unlink(f"/tmp/perf-{pid}.map")
454439

455440
def run_perf(self, script_dir, script, activate_trampoline=True):
456441
if activate_trampoline:
@@ -529,13 +514,21 @@ def compile_trampolines_for_all_functions():
529514
self.assertIn(f"py::foo_fork:{script}", child_perf_file_contents)
530515
self.assertIn(f"py::bar_fork:{script}", child_perf_file_contents)
531516

517+
# Pre-compiled perf-map entries of a forked process must be
518+
# identical in both the parent and child perf-map files.
532519
perf_file_lines = perf_file_contents.split("\n")
533520
for line in perf_file_lines:
534521
if f"py::foo_fork:{script}" in line or f"py::bar_fork:{script}" in line:
535522
self.assertIn(line, child_perf_file_contents)
536523

537524

538525
def _is_perf_version_at_least(major, minor):
526+
# The output of perf --version looks like "perf version 6.7-3" but
527+
# it can also be perf version "perf version 5.15.143", or even include
528+
# a commit hash in the version string, like "6.12.9.g242e6068fd5c"
529+
#
530+
# PermissionError is raised if perf does not exist on the Windows Subsystem
531+
# for Linux, see #134987
539532
try:
540533
output = subprocess.check_output(["perf", "--version"], text=True)
541534
except (subprocess.CalledProcessError, FileNotFoundError, PermissionError):

0 commit comments

Comments
 (0)