Skip to content

Commit 993bbb2

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Move immortalize functions to pre-fork model build
Summary: This takes our existing immortal code object support and moves it into the pre-fork build configuration. Reviewed By: alexmalyshev Differential Revision: D109072395 fbshipit-source-id: aa8d62f8bba91676f6e18f7e1d4fc00f8d7f8b29
1 parent 52fb323 commit 993bbb2

13 files changed

Lines changed: 123 additions & 21 deletions

File tree

cinderx/Common/define.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,18 @@ constexpr bool kFreeThreadedBuild =
3131
#else
3232
false;
3333
#endif
34+
35+
// True when CinderX is built for the prefork (fork-and-exec) process model,
36+
// i.e. with the ENABLE_PREFORK_MODEL build flag. In this mode some behaviors
37+
// that would otherwise be runtime options are forced on at compile time -- e.g.
38+
// JIT-compiled functions are always immortalized, avoiding refcount churn that
39+
// would otherwise be copied-on-write across forked worker processes.
40+
//
41+
// Prefer branching on this constexpr over #ifdef ENABLE_PREFORK_MODEL so the
42+
// guarded code still gets type-checked in every build configuration.
43+
constexpr bool kPreforkModel =
44+
#ifdef ENABLE_PREFORK_MODEL
45+
true;
46+
#else
47+
false;
48+
#endif

cinderx/Jit/config.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,6 @@ struct Config {
212212
// List of function name patterns for which to capture compilation times.
213213
std::vector<std::string> capture_compilation_times_for;
214214

215-
// Option to force compiled functions to be immortalized. By default a
216-
// CompiledFunction's timetime will be tied to a function via a reference put
217-
// in the function's __dict__. When we force CompiledFunction's to always be
218-
// immortalized no such reference will be created and the CompiledFunction
219-
// will be set to be immortal and never collected.
220-
bool immortalize_compiled_functions{false};
221-
222215
// Use stable sentinel pointers in output (for deterministic test output).
223216
bool use_stable_pointers{false};
224217

cinderx/Jit/context.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -718,8 +718,9 @@ Ref<CompiledFunction> Context::makeCompiledFunction(
718718
if (outer_it != code_outer_funcs_.end() && outer_it->second != func) {
719719
outer = outer_it->second;
720720
}
721-
bool immortal = getConfig().immortalize_compiled_functions ||
722-
(func != nullptr && _Py_IsImmortal(func)) ||
721+
// In the prefork model JIT-compiled functions are always immortalized (see
722+
// kPreforkModel); otherwise they're only immortal if the owning function is.
723+
bool immortal = kPreforkModel || (func != nullptr && _Py_IsImmortal(func)) ||
723724
(outer != nullptr && _Py_IsImmortal(outer));
724725
auto compiled = CompiledFunction::create(std::move(compiled_func), immortal);
725726
if (compiled == nullptr) {

cinderx/Jit/pyjit.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -703,12 +703,6 @@ FlagProcessor initFlagProcessor() {
703703
getMutableConfig().compile_perf_trampoline_prefork,
704704
"Compile perf trampoline pre-fork");
705705

706-
flag_processor.addOption(
707-
"cinderx-jit-immortalize-compiled-functions",
708-
"CINDERX_JIT_IMMORTALIZE_COMPILED_FUNCTIONS",
709-
getMutableConfig().immortalize_compiled_functions,
710-
"Always immortalize CompiledFunction objects");
711-
712706
flag_processor.addOption(
713707
"cinderx-jit-max-code-size",
714708
"CINDERX_JIT_MAX_CODE_SIZE",

cinderx/PythonLib/cinderx/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ def is_supported_runtime() -> bool:
7474
install_frame_evaluator,
7575
is_frame_evaluator_installed,
7676
is_immortal,
77+
is_prefork_build,
7778
remove_frame_evaluator,
7879
set_adaptive_delay,
7980
strict_module_patch,
@@ -485,6 +486,9 @@ def is_immortal(obj: object) -> bool:
485486
"Can't answer whether an object is mortal or immortal from Python code"
486487
)
487488

489+
def is_prefork_build() -> bool:
490+
return False
491+
488492
def remove_frame_evaluator() -> None:
489493
pass
490494

cinderx/PythonLib/cinderx/test_support.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ def skip_if_jit(reason: str) -> Callable[[Callable[..., None]], Callable[..., No
135135
return passIf(cinderx.jit.is_enabled(), reason)
136136

137137

138+
def skip_if_prefork(
139+
reason: str = "Behavior intentionally differs in prefork builds (e.g. compiled functions are always immortalized)",
140+
) -> Callable[[Callable[..., None]], Callable[..., None]]:
141+
return passIf(cinderx.is_prefork_build(), reason)
142+
143+
138144
def skip_if_ft(reason: str) -> Callable[[Callable[..., None]], Callable[..., None]]:
139145
return passIf(FREE_THREADING_BUILD, reason)
140146

cinderx/PythonLib/test_cinderx/test_cinderjit.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
passUnless,
3333
run_in_subprocess,
3434
skip_if_ft,
35+
skip_if_prefork,
3536
skip_test_if_oss,
3637
skip_unless_jit,
3738
subprocess_env,
@@ -2541,6 +2542,10 @@ def test_jit_unsuppress(self) -> None:
25412542
jit_unsuppress(is_jit_compiled)
25422543

25432544
@passIf(not cinderx.jit.is_enabled(), "only relevant when the JIT is enabled")
2545+
@skip_if_prefork(
2546+
"Prefork builds immortalize compiled functions and do not publish "
2547+
"__cinderx_compiled_func__, so the function never deopts"
2548+
)
25442549
def test_compiled_code_ref(self):
25452550
self.assertNotIn("__cinderx_compiled_func__", compiled_code_func.__dict__)
25462551
cinder_support.failUnlessJITCompiled(compiled_code_func)
@@ -2553,6 +2558,10 @@ def test_compiled_code_ref(self):
25532558
self.assertFalse(cinderx.jit.is_jit_compiled(compiled_code_func))
25542559

25552560
@passIf(not cinderx.jit.is_enabled(), "only relevant when the JIT is enabled")
2561+
@skip_if_prefork(
2562+
"Prefork builds immortalize compiled functions and do not publish "
2563+
"__cinderx_compiled_func__, so the function never deopts"
2564+
)
25562565
def test_nested_compiled_code_ref(self):
25572566
# CompiledCode should be re-used for nested functions, even if the outer
25582567
# function is never compiled.
@@ -2573,6 +2582,10 @@ def test_nested_compiled_code_ref(self):
25732582
)
25742583

25752584
@passIf(not cinderx.jit.is_enabled(), "only relevant when the JIT is enabled")
2585+
@skip_if_prefork(
2586+
"Prefork builds immortalize compiled functions and do not publish "
2587+
"__cinderx_compiled_func__, so the function never deopts"
2588+
)
25762589
def test_nested_compiled_code_ref_outer_destroyed(self):
25772590
d = {}
25782591
exec(

cinderx/PythonLib/test_cinderx/test_deferred_cleanup.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
import unittest
1010
from pathlib import Path
1111

12-
from cinderx.test_support import ENCODING, skip_unless_jit, subprocess_env
12+
from cinderx.test_support import (
13+
ENCODING,
14+
skip_if_prefork,
15+
skip_unless_jit,
16+
subprocess_env,
17+
)
1318

1419

1520
# Each subprocess must use the allocator that actually unmaps freed code
@@ -24,6 +29,10 @@
2429

2530

2631
@skip_unless_jit("Exercises JIT-compiled code lifetime")
32+
@skip_if_prefork(
33+
"Prefork builds always immortalize compiled functions, so the mortal "
34+
"deferred-cleanup/deopt lifecycle these tests exercise does not apply"
35+
)
2736
class DeferredCleanupTest(unittest.TestCase):
2837
"""
2938
Tests for the deferred freeing of JIT-compiled code (the

cinderx/RuntimeTests/main.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,16 @@ void registerCinderX() {
210210
#endif
211211
}
212212

213+
// In the prefork-model build CinderX intentionally immortalizes JIT-compiled
214+
// objects, so they are never freed and LeakSanitizer reports them as leaks at
215+
// exit, failing the test binary even though every gtest passes. Turn leak
216+
// checking off for that build only; non-prefork builds keep leak detection.
217+
// kPreforkModel is a constexpr, so this folds to a constant return as required
218+
// by __lsan_is_turned_off().
219+
extern "C" __attribute__((used)) int __lsan_is_turned_off() {
220+
return int{kPreforkModel};
221+
}
222+
213223
int main(int argc, char* argv[]) {
214224
#ifdef CINDERX_RUNTIME_TESTS_PYTHONPATH_PACKAGE
215225
// OSS path: point PYTHONPATH at the in-tree cinderx package so

cinderx/TestScripts/cinder_test_runner.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,9 @@ def manage_worker(
274274
worker.wait()
275275

276276

277-
def _computeSkipTests(huntrleaks, use_rr=False) -> Tuple[Set[str], Set[str]]:
277+
def _computeSkipTests(
278+
huntrleaks, use_rr=False, extra_skip_files=None
279+
) -> Tuple[Set[str], Set[str]]:
278280
skip_list_files = ["devserver_skip_tests.txt", "cinder_skip_test.txt"]
279281

280282
version = "".join(str(v) for v in sys.version_info[:2])
@@ -288,6 +290,9 @@ def _computeSkipTests(huntrleaks, use_rr=False) -> Tuple[Set[str], Set[str]]:
288290
if use_rr:
289291
skip_list_files.append("rr_skip_tests.txt")
290292

293+
if extra_skip_files:
294+
skip_list_files.extend(extra_skip_files)
295+
291296
try:
292297
import cinderjit # noqa: F401
293298

@@ -354,8 +359,10 @@ def _select_tests(exclude: Set[str]) -> List[str]:
354359
return tests
355360

356361

357-
def list_tests(huntrleaks, use_rr):
358-
skip_modules, skip_patterns = _computeSkipTests(huntrleaks, use_rr)
362+
def list_tests(huntrleaks, use_rr, extra_skip_files=None):
363+
skip_modules, skip_patterns = _computeSkipTests(
364+
huntrleaks, use_rr, extra_skip_files
365+
)
359366

360367
return _select_tests(skip_modules)
361368

@@ -376,6 +383,7 @@ def __init__(
376383
json_summary_file: str | None,
377384
failfast: bool,
378385
expected_failures: list[str],
386+
skip_lists: list[str] | None = None,
379387
):
380388
self._cinder_regr_runner_logfile = logfile
381389
self._success_on_test_errors = success_on_test_errors
@@ -403,7 +411,9 @@ def __init__(
403411
# False, False => quiet, pgo
404412
self._logger = libregrtest_logger.Logger(self._results, False, False)
405413

406-
skip_modules, skip_patterns = _computeSkipTests(self._huntrleaks, self._use_rr)
414+
skip_modules, skip_patterns = _computeSkipTests(
415+
self._huntrleaks, self._use_rr, skip_lists
416+
)
407417

408418
if tests is None:
409419
tests = _select_tests(skip_modules)
@@ -866,7 +876,7 @@ def worker_main(args):
866876

867877
def dispatcher_main(args):
868878
if args.list:
869-
tests = list_tests(args.huntrleaks, args.use_rr)
879+
tests = list_tests(args.huntrleaks, args.use_rr, args.skip_list)
870880
tests.sort()
871881
print(json.dumps(tests, indent=0))
872882
sys.exit(0)
@@ -901,6 +911,7 @@ def dispatcher_main(args):
901911
args.json_summary_file,
902912
args.failfast,
903913
args.expected_failures,
914+
args.skip_list,
904915
)
905916
print(f"Spawning {num_workers} workers")
906917
test_runner.run()
@@ -1033,6 +1044,14 @@ def main():
10331044
action="append",
10341045
help="A file which specifies expected failures for this test run.",
10351046
)
1047+
dispatcher_parser.add_argument(
1048+
"--skip-list",
1049+
action="append",
1050+
help="A file listing extra tests to skip for this run, in the same "
1051+
"format as the checked-in *_skip_tests.txt files. Use to skip tests "
1052+
"for a specific build configuration (e.g. prefork-model) without "
1053+
"affecting other builds. Can be supplied multiple times.",
1054+
)
10361055
dispatcher_parser.add_argument("rest", nargs=argparse.REMAINDER)
10371056
dispatcher_parser.set_defaults(func=dispatcher_main)
10381057

0 commit comments

Comments
 (0)