Skip to content

Commit b0bf9b1

Browse files
alexmalyshevfacebook-github-bot
authored andcommitted
Support running test_jit_disable.py functions multiple times
Summary: The test functions here need to be explicitly uncompiled (and in one case, unsuppressed) otherwise their compiled state will stick around on the next test run. This happens when running the refleak tests. `force_uncompile` needs to get stronger about forcefully dropping all compilation state for a function. Its current behavior is effectively just a deopt, if we create another function with the same code object we'll reopt it immediately upon calling lazy_compile(), which is not what we want here. Reviewed By: czardoz Differential Revision: D79690872 fbshipit-source-id: 52f14546cf67cb0f7ac6f3a9bd1b8fe55f35d256
1 parent 400eaef commit b0bf9b1

4 files changed

Lines changed: 41 additions & 0 deletions

File tree

cinderx/Jit/context.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ Context::CompilationResult Context::compilePreloader(
3535
return result;
3636
}
3737

38+
void Context::uncompile(BorrowedRef<PyFunctionObject> func) {
39+
deoptFuncImpl(func);
40+
compiled_codes_.erase(CompilationKey{func});
41+
}
42+
3843
bool Context::deoptFunc(BorrowedRef<PyFunctionObject> func) {
3944
if (deoptFuncImpl(func)) {
4045
deopted_funcs_.emplace(func);
@@ -104,6 +109,9 @@ void Context::funcModified(BorrowedRef<PyFunctionObject> func) {
104109
void Context::funcDestroyed(BorrowedRef<PyFunctionObject> func) {
105110
compiled_funcs_.erase(func);
106111
deopted_funcs_.erase(func);
112+
113+
// This doesn't modify compiled_codes_, so if this is a nested function it can
114+
// easily be reopted later.
107115
}
108116

109117
Context::CompilationResult Context::compilePreloaderImpl(

cinderx/Jit/context.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ struct CompilationKey {
2626
PyObject* builtins;
2727
PyObject* globals;
2828

29+
explicit CompilationKey(BorrowedRef<PyFunctionObject> func)
30+
: code{func->func_code},
31+
builtins{func->func_builtins},
32+
globals{func->func_globals} {}
33+
2934
CompilationKey(PyObject* code, PyObject* builtins, PyObject* globals)
3035
: code(code), builtins(builtins), globals(globals) {}
3136

@@ -72,6 +77,11 @@ class Context {
7277
BorrowedRef<PyFunctionObject> func,
7378
const hir::Preloader& preloader);
7479

80+
/*
81+
* Fully remove all effects of compilation from a function.
82+
*/
83+
void uncompile(BorrowedRef<PyFunctionObject> func);
84+
7585
/*
7686
* De-optimize a function by setting it to run through the interpreter if it
7787
* had been previously JIT-compiled.

cinderx/Jit/pyjit.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,6 +1382,9 @@ PyObject* force_uncompile(PyObject* /* self */, PyObject* arg) {
13821382
// "Destroy" the function from the perspective of the JIT, effectively erasing
13831383
// all traces of it from the metadata.
13841384
funcDestroyed(func);
1385+
if (jit_ctx != nullptr) {
1386+
jit_ctx->uncompile(func);
1387+
}
13851388

13861389
Py_RETURN_TRUE;
13871390
}

cinderx/PythonLib/test_cinderx/test_jit_disable.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
disable as disable_jit,
99
enable as enable_jit,
1010
force_compile,
11+
force_uncompile,
1112
INSTALLED,
1213
is_enabled as is_jit_enabled,
1314
is_jit_compiled,
1415
jit_suppress,
16+
jit_unsuppress,
1517
lazy_compile,
1618
pause as pause_jit,
1719
)
@@ -45,6 +47,8 @@ def foo(a, b):
4547
enable_jit()
4648
self.assertTrue(is_jit_compiled(foo))
4749

50+
force_uncompile(foo)
51+
4852
def test_suppress_and_reopt(self) -> None:
4953
def foo(a, b):
5054
return a + b
@@ -61,6 +65,8 @@ def foo(a, b):
6165
enable_jit()
6266
self.assertFalse(is_jit_compiled(foo))
6367

68+
jit_unsuppress(foo)
69+
6470
def test_disable_then_deopt(self) -> None:
6571
def foo(a, b):
6672
return a + b
@@ -77,6 +83,8 @@ def foo(a, b):
7783
enable_jit()
7884
self.assertTrue(is_jit_compiled(foo))
7985

86+
force_uncompile(foo)
87+
8088
def test_already_disabled(self) -> None:
8189
def foo(a, b):
8290
return a + b
@@ -93,6 +101,8 @@ def foo(a, b):
93101
enable_jit()
94102
self.assertTrue(is_jit_compiled(foo))
95103

104+
force_uncompile(foo)
105+
96106
def test_already_enabled(self) -> None:
97107
def foo(a, b):
98108
return a + b
@@ -109,6 +119,8 @@ def foo(a, b):
109119
enable_jit()
110120
self.assertTrue(is_jit_compiled(foo))
111121

122+
force_uncompile(foo)
123+
112124
def test_compile_new_after_reenable(self) -> None:
113125
disable_jit(deopt_all=True)
114126

@@ -123,6 +135,8 @@ def foo(a, b):
123135
force_compile(foo)
124136
self.assertTrue(is_jit_compiled(foo))
125137

138+
force_uncompile(foo)
139+
126140
def test_pause(self) -> None:
127141
def foo(a, b):
128142
return a + b
@@ -148,6 +162,8 @@ def foo(a, b):
148162
self.assertTrue(is_jit_enabled())
149163
self.assertTrue(is_jit_compiled(foo))
150164

165+
force_uncompile(foo)
166+
151167
def test_pause_nested(self) -> None:
152168
def foo(a, b):
153169
return a + b
@@ -169,6 +185,8 @@ def foo(a, b):
169185
self.assertTrue(is_jit_enabled())
170186
self.assertTrue(is_jit_compiled(foo))
171187

188+
force_uncompile(foo)
189+
172190
def test_pause_between_lazy_compile(self) -> None:
173191
def foo(a, b):
174192
return a + b
@@ -185,6 +203,8 @@ def foo(a, b):
185203
foo(3, 4)
186204
self.assertTrue(is_jit_compiled(foo))
187205

206+
force_uncompile(foo)
207+
188208

189209
if __name__ == "__main__":
190210
unittest.main()

0 commit comments

Comments
 (0)