From 48c50ff1a22f086c302c52a70eb9912d76c66f91 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 20 Nov 2024 08:11:25 -0800 Subject: [PATCH] GH-126892: Reset warmup counters when JIT compiling code (GH-126893) --- Lib/test/support/strace_helper.py | 5 +- Lib/test/test_capi/test_opt.py | 50 ++++++++----------- ...-11-15-16-39-37.gh-issue-126892.QR6Yo3.rst | 2 + Python/bytecodes.c | 14 ++++-- Python/executor_cases.c.h | 3 ++ Python/generated_cases.c.h | 13 +++-- 6 files changed, 48 insertions(+), 39 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-11-15-16-39-37.gh-issue-126892.QR6Yo3.rst diff --git a/Lib/test/support/strace_helper.py b/Lib/test/support/strace_helper.py index 90d4b5bccb6..7fb4581b2a7 100644 --- a/Lib/test/support/strace_helper.py +++ b/Lib/test/support/strace_helper.py @@ -91,7 +91,10 @@ def _make_error(reason, details): res, cmd_line = run_python_until_end( "-c", textwrap.dedent(code), - __run_using_command=[_strace_binary] + strace_flags) + __run_using_command=[_strace_binary] + strace_flags, + # Don't want to trace our JIT's own mmap and mprotect calls: + PYTHON_JIT="0", + ) except OSError as err: return _make_error("Caught OSError", err) diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index edd83872e57..4cf9b66170c 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1385,20 +1385,17 @@ class Foo: guard_type_version_count = opnames.count("_GUARD_TYPE_VERSION") self.assertEqual(guard_type_version_count, 1) - def test_guard_type_version_not_removed(self): - """ - Verify that the guard type version is not removed if we modify the class - """ + def test_guard_type_version_removed_invalidation(self): def thing(a): x = 0 - for i in range(TIER2_THRESHOLD + 100): + for i in range(TIER2_THRESHOLD * 2 + 1): x += a.attr - # for the first (TIER2_THRESHOLD + 90) iterations we set the attribute on this dummy function which shouldn't - # trigger the type watcher - # then after for the next 10 it should trigger it and stop optimizing - # Note that the code needs to be in this weird form so it's optimized inline without any control flow - setattr((Foo, Bar)[i < TIER2_THRESHOLD + 90], "attr", 2) + # The first TIER2_THRESHOLD iterations we set the attribute on + # this dummy class, which shouldn't trigger the type watcher. + # Note that the code needs to be in this weird form so it's + # optimized inline without any control flow: + setattr((Bar, Foo)[i == TIER2_THRESHOLD + 1], "attr", 2) x += a.attr return x @@ -1410,24 +1407,21 @@ class Bar: res, ex = self._run_with_optimizer(thing, Foo()) opnames = list(iter_opnames(ex)) - self.assertIsNotNone(ex) - self.assertEqual(res, (TIER2_THRESHOLD * 2) + 219) - guard_type_version_count = opnames.count("_GUARD_TYPE_VERSION") - self.assertEqual(guard_type_version_count, 2) + self.assertEqual(res, TIER2_THRESHOLD * 6 + 1) + call = opnames.index("_CALL_BUILTIN_FAST") + load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call) + load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call) + self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1) + self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1) - - @unittest.expectedFailure - def test_guard_type_version_not_removed_escaping(self): - """ - Verify that the guard type version is not removed if have an escaping function - """ + def test_guard_type_version_removed_escaping(self): def thing(a): x = 0 - for i in range(100): + for i in range(TIER2_THRESHOLD): x += a.attr - # eval should be escaping and so should cause optimization to stop and preserve both type versions + # eval should be escaping eval("None") x += a.attr return x @@ -1437,12 +1431,12 @@ class Foo: res, ex = self._run_with_optimizer(thing, Foo()) opnames = list(iter_opnames(ex)) self.assertIsNotNone(ex) - self.assertEqual(res, 200) - guard_type_version_count = opnames.count("_GUARD_TYPE_VERSION") - # Note: This will actually be 1 for noe - # https://github.com/python/cpython/pull/119365#discussion_r1626220129 - self.assertEqual(guard_type_version_count, 2) - + self.assertEqual(res, TIER2_THRESHOLD * 2) + call = opnames.index("_CALL_BUILTIN_FAST_WITH_KEYWORDS") + load_attr_top = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", 0, call) + load_attr_bottom = opnames.index("_LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", call) + self.assertEqual(opnames[:load_attr_top].count("_GUARD_TYPE_VERSION"), 1) + self.assertEqual(opnames[call:load_attr_bottom].count("_CHECK_VALIDITY"), 1) def test_guard_type_version_executor_invalidated(self): """ diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-15-16-39-37.gh-issue-126892.QR6Yo3.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-15-16-39-37.gh-issue-126892.QR6Yo3.rst new file mode 100644 index 00000000000..db3c398e5db --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-15-16-39-37.gh-issue-126892.QR6Yo3.rst @@ -0,0 +1,2 @@ +Require cold or invalidated code to "warm up" before being JIT compiled +again. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index c85b49842da..029bdb2d9f5 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2624,15 +2624,16 @@ dummy_func( } _PyExecutorObject *executor; int optimized = _PyOptimizer_Optimize(frame, start, stack_pointer, &executor, 0); - ERROR_IF(optimized < 0, error); - if (optimized) { + if (optimized <= 0) { + this_instr[1].counter = restart_backoff_counter(counter); + ERROR_IF(optimized < 0, error); + } + else { + this_instr[1].counter = initial_jump_backoff_counter(); assert(tstate->previous_executor == NULL); tstate->previous_executor = Py_None; GOTO_TIER_TWO(executor); } - else { - this_instr[1].counter = restart_backoff_counter(counter); - } } else { ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter); @@ -4875,6 +4876,9 @@ dummy_func( tstate->previous_executor = (PyObject *)current_executor; GOTO_TIER_ONE(target); } + else { + exit->temperature = initial_temperature_backoff_counter(); + } } exit->executor = executor; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 2c2a09adf28..e9c9ed2b5da 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -5750,6 +5750,9 @@ tstate->previous_executor = (PyObject *)current_executor; GOTO_TIER_ONE(target); } + else { + exit->temperature = initial_temperature_backoff_counter(); + } } exit->executor = executor; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 15308d6f1f7..4d89753bae7 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5092,15 +5092,18 @@ _PyFrame_SetStackPointer(frame, stack_pointer); int optimized = _PyOptimizer_Optimize(frame, start, stack_pointer, &executor, 0); stack_pointer = _PyFrame_GetStackPointer(frame); - if (optimized < 0) goto error; - if (optimized) { + if (optimized <= 0) { + this_instr[1].counter = restart_backoff_counter(counter); + if (optimized < 0) goto error; + } + else { + _PyFrame_SetStackPointer(frame, stack_pointer); + this_instr[1].counter = initial_jump_backoff_counter(); + stack_pointer = _PyFrame_GetStackPointer(frame); assert(tstate->previous_executor == NULL); tstate->previous_executor = Py_None; GOTO_TIER_TWO(executor); } - else { - this_instr[1].counter = restart_backoff_counter(counter); - } } else { ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter);