Skip to content

Commit c1a0397

Browse files
alexmalyshevfacebook-github-bot
authored andcommitted
Reset function entrypoints when we change compile_after_n_calls
Summary: Making sure that this affects existing functions and not just functions that are created afterwards. Consolidating code between `auto` and `compile_after_n_calls` here, added a check that the call limit isn't out-of-bounds. Reviewed By: czardoz Differential Revision: D82479038 fbshipit-source-id: 909d9f4d044fa985bfab2849a967cc1a3ca3bb88
1 parent 3867cb5 commit c1a0397

3 files changed

Lines changed: 100 additions & 11 deletions

File tree

cinderx/Jit/pyjit.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,12 +1339,22 @@ PyObject* enable_jit(PyObject* /* self */, PyObject* /* arg */) {
13391339
Py_RETURN_NONE;
13401340
}
13411341

1342+
void compile_after_n_calls_impl(uint32_t calls) {
1343+
getMutableConfig().auto_jit_threshold = calls;
1344+
1345+
// Schedule all pre-existing functions for compilation.
1346+
walkFunctionObjects(
1347+
[](BorrowedRef<PyFunctionObject> func) { scheduleJitCompile(func); });
1348+
1349+
JIT_DLOG("Configuring JIT to compile functions after {} calls", calls);
1350+
}
1351+
13421352
PyObject* compile_after_n_calls(PyObject* /* self */, PyObject* arg) {
13431353
Py_ssize_t calls = -1;
13441354
if (!PyArg_Parse(arg, "n:compile_after_n_calls", &calls)) {
13451355
return nullptr;
13461356
}
1347-
if (calls < 0) {
1357+
if (calls < 0 || calls > std::numeric_limits<uint32_t>::max()) {
13481358
PyErr_Format(
13491359
PyExc_ValueError,
13501360
"Cannot configure JIT to compile functions after '%zd' calls",
@@ -1358,21 +1368,14 @@ PyObject* compile_after_n_calls(PyObject* /* self */, PyObject* arg) {
13581368
return nullptr;
13591369
}
13601370

1361-
getMutableConfig().auto_jit_threshold = calls;
1362-
JIT_DLOG("Configuring JIT to compile functions after {} calls", calls);
1371+
compile_after_n_calls_impl(calls);
13631372

13641373
Py_RETURN_NONE;
13651374
}
13661375

13671376
PyObject* auto_jit(PyObject* /* self */, PyObject* /* arg */) {
13681377
// Default value that works well for most applications.
1369-
constexpr size_t kThreshold = 1000;
1370-
1371-
getMutableConfig().auto_jit_threshold = kThreshold;
1372-
1373-
JIT_DLOG(
1374-
"Configuring JIT to compile functions automatically using default "
1375-
"behavior");
1378+
compile_after_n_calls_impl(1000);
13761379

13771380
Py_RETURN_NONE;
13781381
}

cinderx/PythonLib/test_cinderx/test_cinderjit.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2572,6 +2572,8 @@ def test_compile_after_n_calls(self) -> None:
25722572
compile_after_n_calls(is_jit_compiled)
25732573
with self.assertRaises(ValueError):
25742574
compile_after_n_calls(-1)
2575+
with self.assertRaises(ValueError):
2576+
compile_after_n_calls(10_000_000_000)
25752577
with self.assertRaises(ValueError):
25762578
compile_after_n_calls(0)
25772579

cinderx/PythonLib/test_cinderx/test_jit_disable.py

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,12 @@ def foo(a, b):
207207
# detection.
208208
force_uncompile(foo)
209209

210-
def test_default(self) -> None:
210+
def test_compile_no_config(self) -> None:
211+
"""
212+
Test how code behaves when it forces compilation without any other
213+
configuration or options enabled.
214+
"""
215+
211216
with tempfile.TemporaryDirectory() as tmp_dir:
212217
code = textwrap.dedent("""
213218
import cinderx.jit
@@ -230,10 +235,17 @@ def inc(x):
230235
)
231236

232237
def test_auto(self) -> None:
238+
"""
239+
Basic test for cinderx.jit.auto().
240+
"""
241+
233242
with tempfile.TemporaryDirectory() as tmp_dir:
234243
code = textwrap.dedent("""
235244
import cinderx.jit
236245
246+
def predefined(x):
247+
return x + x
248+
237249
cinderx.jit.auto()
238250
239251
def inc(x):
@@ -257,7 +269,44 @@ def inc(x):
257269
env={"PYTHONPATH": CINDERX_PATH},
258270
)
259271

272+
def test_auto_predefined(self) -> None:
273+
"""
274+
Test that cinderx.jit.auto() works for functions that were defined
275+
before it was called.
276+
"""
277+
278+
with tempfile.TemporaryDirectory() as tmp_dir:
279+
code = textwrap.dedent("""
280+
import cinderx.jit
281+
282+
def predefined(x):
283+
return x + x
284+
285+
cinderx.jit.auto()
286+
287+
assert not cinderx.jit.is_jit_compiled(predefined)
288+
for i in range(1000):
289+
predefined(i)
290+
assert not cinderx.jit.is_jit_compiled(predefined)
291+
292+
predefined(1001)
293+
assert cinderx.jit.is_jit_compiled(predefined)
294+
""")
295+
296+
test_file = Path(tmp_dir) / "mod.py"
297+
test_file.write_text(code)
298+
299+
subprocess.run(
300+
[sys.executable, str(test_file)],
301+
check=True,
302+
env={"PYTHONPATH": CINDERX_PATH},
303+
)
304+
260305
def test_compile_after_n_calls(self) -> None:
306+
"""
307+
Basic test for cinderx.jit.compile_after_n_calls().
308+
"""
309+
261310
with tempfile.TemporaryDirectory() as tmp_dir:
262311
code = textwrap.dedent("""
263312
import cinderx.jit
@@ -275,6 +324,8 @@ def inc(x):
275324
inc(3)
276325
assert cinderx.jit.is_jit_compiled(inc)
277326
327+
# Change the setting and see it takes affect.
328+
278329
cinderx.jit.compile_after_n_calls(5)
279330
280331
def dec(x):
@@ -301,6 +352,39 @@ def dec(x):
301352
env={"PYTHONPATH": CINDERX_PATH},
302353
)
303354

355+
def test_compile_after_n_calls_predefined(self) -> None:
356+
"""
357+
Test that cinderx.jit.compile_after_n_calls() works for functions that
358+
were defined before it was called.
359+
"""
360+
361+
with tempfile.TemporaryDirectory() as tmp_dir:
362+
code = textwrap.dedent("""
363+
import cinderx.jit
364+
365+
def predefined(x):
366+
return x + x
367+
368+
cinderx.jit.compile_after_n_calls(2)
369+
370+
assert not cinderx.jit.is_jit_compiled(predefined)
371+
predefined(1)
372+
predefined(2)
373+
assert not cinderx.jit.is_jit_compiled(predefined)
374+
375+
predefined(3)
376+
assert cinderx.jit.is_jit_compiled(predefined)
377+
""")
378+
379+
test_file = Path(tmp_dir) / "mod.py"
380+
test_file.write_text(code)
381+
382+
subprocess.run(
383+
[sys.executable, str(test_file)],
384+
check=True,
385+
env={"PYTHONPATH": CINDERX_PATH},
386+
)
387+
304388

305389
if __name__ == "__main__":
306390
unittest.main()

0 commit comments

Comments
 (0)