Skip to content

Commit 43d526a

Browse files
Gamal Sallamfacebook-github-bot
authored andcommitted
Fix some nits and add new unit tests
Summary: As titled. To be on sync with python/cpython#109666. Reviewed By: czardoz Differential Revision: D50329585 fbshipit-source-id: d4c52143216a1d9970196ad71ef656098be50120
1 parent 6f02c7f commit 43d526a

4 files changed

Lines changed: 123 additions & 13 deletions

File tree

CinderX/test_cinderx/test_perf_profiler.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,82 @@ def baz(n):
340340
self.assertNotIn(f"py::bar:{script}", stdout)
341341
self.assertNotIn(f"py::baz:{script}", stdout)
342342

343+
def test_pre_fork_compile(self):
344+
code = """if 1:
345+
import sys
346+
import os
347+
import sysconfig
348+
from _testinternalcapi import (
349+
compile_perf_trampoline_entry,
350+
perf_trampoline_set_persist_after_fork,
351+
)
352+
353+
def foo_fork():
354+
pass
355+
356+
def bar_fork():
357+
foo_fork()
358+
359+
def foo():
360+
pass
361+
362+
def bar():
363+
foo()
364+
365+
def compile_trampolines_for_all_functions():
366+
perf_trampoline_set_persist_after_fork(1)
367+
for _, obj in globals().items():
368+
if callable(obj) and hasattr(obj, '__code__'):
369+
compile_perf_trampoline_entry(obj.__code__)
370+
371+
if __name__ == "__main__":
372+
compile_trampolines_for_all_functions()
373+
pid = os.fork()
374+
if pid == 0:
375+
print(os.getpid())
376+
bar_fork()
377+
else:
378+
bar()
379+
"""
380+
381+
with temp_dir() as script_dir:
382+
script = make_script(script_dir, "perftest", code)
383+
with subprocess.Popen(
384+
[sys.executable, "-Xperf", script],
385+
universal_newlines=True,
386+
stderr=subprocess.PIPE,
387+
stdout=subprocess.PIPE,
388+
) as process:
389+
stdout, stderr = process.communicate()
390+
391+
self.assertEqual(process.returncode, 0)
392+
self.assertNotIn("Error:", stderr)
393+
child_pid = int(stdout.strip())
394+
perf_file = pathlib.Path(f"/tmp/perf-{process.pid}.map")
395+
perf_child_file = pathlib.Path(f"/tmp/perf-{child_pid}.map")
396+
self.assertTrue(perf_file.exists())
397+
self.assertTrue(perf_child_file.exists())
398+
399+
perf_file_contents = perf_file.read_text()
400+
self.assertIn(f"py::foo:{script}", perf_file_contents)
401+
self.assertIn(f"py::bar:{script}", perf_file_contents)
402+
self.assertIn(f"py::foo_fork:{script}", perf_file_contents)
403+
self.assertIn(f"py::bar_fork:{script}", perf_file_contents)
404+
405+
child_perf_file_contents = perf_child_file.read_text()
406+
self.assertIn(f"py::foo_fork:{script}", child_perf_file_contents)
407+
self.assertIn(f"py::bar_fork:{script}", child_perf_file_contents)
408+
409+
# Pre-compiled perf-map entries of a forked process must be
410+
# identical in both the parent and child perf-map files.
411+
perf_file_lines = perf_file_contents.split("\n")
412+
for line in perf_file_lines:
413+
if (
414+
f"py::foo_fork:{script}" in line
415+
or f"py::bar_fork:{script}" in line
416+
):
417+
self.assertIn(line, child_perf_file_contents)
418+
343419

344420
if __name__ == "__main__":
345421
unittest.main()

Modules/_testinternalcapi.c

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,36 @@ perf_map_state_teardown(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored))
460460
Py_RETURN_NONE;
461461
}
462462

463+
static PyObject *
464+
compile_perf_trampoline_entry(PyObject *self, PyObject *args)
465+
{
466+
PyObject *co;
467+
if (!PyArg_ParseTuple(args, "O!", &PyCode_Type, &co)) {
468+
return NULL;
469+
}
470+
int ret = PyUnstable_PerfTrampoline_CompileCode((PyCodeObject *)co);
471+
if (ret != 0) {
472+
PyErr_SetString(PyExc_AssertionError, "Failed to compile trampoline");
473+
return NULL;
474+
}
475+
return PyLong_FromLong(ret);
476+
}
477+
478+
static PyObject *
479+
perf_trampoline_set_persist_after_fork(PyObject *self, PyObject *args)
480+
{
481+
int enable;
482+
if (!PyArg_ParseTuple(args, "i", &enable)) {
483+
return NULL;
484+
}
485+
int ret = PyUnstable_PerfTrampoline_SetPersistAfterFork(enable);
486+
if (ret == 0) {
487+
PyErr_SetString(PyExc_AssertionError, "Failed to set persist_after_fork");
488+
return NULL;
489+
}
490+
return PyLong_FromLong(ret);
491+
}
492+
463493
// These are used in native calling tests, ensure the compiler
464494
// doesn't hide or remove these symbols
465495
__attribute__((used))
@@ -490,6 +520,8 @@ static PyMethodDef TestMethods[] = {
490520
{"test_gc_visit_objects_exit_early", test_gc_visit_objects_exit_early, METH_NOARGS},
491521
{"write_perf_map_entry", write_perf_map_entry, METH_VARARGS},
492522
{"perf_map_state_teardown", perf_map_state_teardown, METH_NOARGS},
523+
{"compile_perf_trampoline_entry", compile_perf_trampoline_entry, METH_VARARGS},
524+
{"perf_trampoline_set_persist_after_fork", perf_trampoline_set_persist_after_fork, METH_VARARGS},
493525
{NULL, NULL} /* sentinel */
494526
};
495527

Objects/perf_trampoline.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,11 +456,11 @@ _PyPerfTrampoline_AfterFork_Child(void)
456456
{
457457
#ifdef PY_HAVE_PERF_TRAMPOLINE
458458
PyUnstable_PerfMapState_Fini();
459-
if (persist_after_fork){
459+
if (persist_after_fork) {
460460
char filename[256];
461461
pid_t parent_pid = getppid();
462462
snprintf(filename, sizeof(filename), "/tmp/perf-%d.map", parent_pid);
463-
if(PyUnstable_CopyPerfMapFile(filename) != 0){
463+
if (PyUnstable_CopyPerfMapFile(filename) != 0) {
464464
return PyStatus_Error("Failed to copy perf map file.");
465465
}
466466
} else {

Python/sysmodule.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,7 +2148,7 @@ PyAPI_FUNC(int) PyUnstable_WritePerfMapEntry(
21482148
#ifndef MS_WINDOWS
21492149
if (perf_map_state.perf_map == NULL) {
21502150
int ret = PyUnstable_PerfMapState_Init();
2151-
if(ret != 0){
2151+
if (ret != 0) {
21522152
return ret;
21532153
}
21542154
}
@@ -2183,28 +2183,30 @@ PyAPI_FUNC(int) PyUnstable_CopyPerfMapFile(const char* parent_filename) {
21832183
}
21842184
if (perf_map_state.perf_map == NULL) {
21852185
int ret = PyUnstable_PerfMapState_Init();
2186-
if(ret != 0){
2186+
if (ret != 0){
21872187
return ret;
21882188
}
21892189
}
21902190
char buf[4096];
21912191
PyThread_acquire_lock(perf_map_state.map_lock, 1);
2192+
int fflush_result = 0, result = 0;
21922193
while (1) {
21932194
size_t bytes_read = fread(buf, 1, sizeof(buf), from);
21942195
size_t bytes_written = fwrite(buf, 1, bytes_read, perf_map_state.perf_map);
2195-
fflush(perf_map_state.perf_map);
2196+
fflush_result = fflush(perf_map_state.perf_map);
21962197

2197-
if (bytes_read < sizeof(buf) && feof(from)) {
2198-
fclose(from);
2199-
PyThread_release_lock(perf_map_state.map_lock);
2200-
return 0;
2198+
if (fflush_result != 0 || bytes_read == 0 || bytes_written < bytes_read) {
2199+
result = -1;
2200+
goto close_and_release;
22012201
}
2202-
if (bytes_read == 0 || bytes_written < bytes_read) {
2203-
fclose(from);
2204-
PyThread_release_lock(perf_map_state.map_lock);
2205-
return -1;
2202+
if (bytes_read < sizeof(buf) && feof(from)) {
2203+
goto close_and_release;
22062204
}
22072205
}
2206+
close_and_release:
2207+
fclose(from);
2208+
PyThread_release_lock(perf_map_state.map_lock);
2209+
return result;
22082210
#endif
22092211
return 0;
22102212
}

0 commit comments

Comments
 (0)