Skip to content

GH-104584: Assorted fixes for the optimizer API. #105683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
markshannon merged 10 commits into from
Jun 19, 2023
Merged

GH-104584: Assorted fixes for the optimizer API. #105683

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2335db6
Fix optimizer API.
markshannon Jun 6, 2023
e4c57d8
Merge branch 'main' into fix-optimizer-api
markshannon Jun 7, 2023
4a2ea94
Add test for long loops
markshannon Jun 7, 2023
06345a6
Address review comments.
markshannon Jun 8, 2023
c4107ae
Merge branch 'main' into fix-optimizer-api
markshannon Jun 8, 2023
a5eed25
Merge branch 'main' into fix-optimizer-api
markshannon Jun 9, 2023
18a4b7e
Clear ENTER_EXECUTOR when deopting code objects.
markshannon Jun 9, 2023
6fac756
Merge branch 'main' into fix-optimizer-api
markshannon Jun 15, 2023
019a233
Fix merge artifact.
markshannon Jun 15, 2023
ff61b56
Remove useless comment
markshannon Jun 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address review comments.
  • Loading branch information
@markshannon
markshannon committed Jun 8, 2023
commit 06345a67010c563a925c22ed7afa7f1b0c3af3ac
35 changes: 22 additions & 13 deletions Lib/test/test_capi/test_misc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1917,6 +1917,23 @@ def func():

class TestOptimizerAPI(unittest.TestCase):

@contextlib.contextmanager
def temporary_optimizer(self, opt):
# Code to acquire resource, e.g.:
Comment thread
markshannon marked this conversation as resolved.
Outdated
_testinternalcapi.set_optimizer(opt)
try:
yield
finally:
_testinternalcapi.set_optimizer(None)

@contextlib.contextmanager
def clear_executors(self, func):
try:
yield
finally:
#Clear executors
func.__code__ = func.__code__.replace()

def test_get_set_optimizer(self):
self.assertEqual(_testinternalcapi.get_optimizer(), None)
opt = _testinternalcapi.get_counter_optimizer()
Expand All @@ -1933,16 +1950,11 @@ def loop():

for repeat in range(5):
opt = _testinternalcapi.get_counter_optimizer()
self.assertEqual(opt.get_count(), 0)
try:
_testinternalcapi.set_optimizer(opt)
with self.temporary_optimizer(opt):
self.assertEqual(opt.get_count(), 0)
loop()
with self.clear_executors(loop):
loop()
self.assertEqual(opt.get_count(), 1000)
finally:
_testinternalcapi.set_optimizer(None)
#Clear executors
loop.__code__ = loop.__code__.replace()

def test_long_loop(self):
"Check that we aren't confused by EXTENDED_ARG"
Expand All @@ -1960,14 +1972,11 @@ def long_loop():
nop(); nop(); nop(); nop(); nop(); nop(); nop(); nop();
nop(); nop(); nop(); nop(); nop(); nop(); nop(); nop();

try:
opt = _testinternalcapi.get_counter_optimizer()
_testinternalcapi.set_optimizer(opt)
opt = _testinternalcapi.get_counter_optimizer()
with self.temporary_optimizer(opt):
self.assertEqual(opt.get_count(), 0)
long_loop()
self.assertEqual(opt.get_count(), 10)
finally:
_testinternalcapi.set_optimizer(None)


if __name__ == "__main__":
Expand Down
11 changes: 7 additions & 4 deletions Python/optimizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@ _PyOptimizer_BackEdge(_PyInterpreterFrame *frame, _Py_CODEUNIT *src, _Py_CODEUNI
goto jump_to_destination;
}
_PyOptimizerObject *opt = interp->optimizer;
_PyExecutorObject *executor;
_PyExecutorObject *executor = NULL;
int err = opt->optimize(opt, frame->f_code, dest, &executor);
if (err <= 0) {
assert(executor == NULL);
if (err < 0) {
Comment thread
markshannon marked this conversation as resolved.
return NULL;
}
Expand All @@ -168,13 +169,15 @@ _PyOptimizer_BackEdge(_PyInterpreterFrame *frame, _Py_CODEUNIT *src, _Py_CODEUNI
if (index < 0) {
/* Out of memory. Don't raise and assume that the
* error will show up elsewhere.
* This might confuse an optimizer as if assumes that
* it has already optimixed this point */
*
* If an optimizer has already produced an executor,
* it might get confused by the executor disappearing,
* but there is not much we can do about that here. */
Py_DECREF(executor);
goto jump_to_destination;
}
insert_executor(frame->f_code, src, index, executor);
assert(frame->prev_instr = src);
assert(frame->prev_instr == src);
return executor->execute(executor, frame, stack_pointer);
jump_to_destination:
frame->prev_instr = dest - 1;
Expand Down