gh-144140: Optimize len for string constants in optimizer#144142
gh-144140: Optimize len for string constants in optimizer#144142Fidget-Spinner merged 6 commits intopython:mainfrom
Conversation
y1yang0
requested review from
Fidget-Spinner,
markshannon,
savannahostrowski and
tomasr8
as code owners
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
I'm not sure if this patch requires a new news item. I'll let the reviewer decide. |
|
Hi, I think there have been some CI changes in the main branch, and this branch is not up to date, which is causing the CI to fail. Could you please sync this branch with main (you can simply click the “Update branch” button above, and please avoid rebasing and force push in the CPython repo) to make the CI pass? |
Lib/test/test_capi/test_opt.py
Outdated
| _, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD) | ||
| self.assertIsNotNone(ex) | ||
| uops = get_opnames(ex) | ||
| print(uops) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The tests failed, but it seems unrelated to this patch. I can't find a way to retry the failed checks. Could a reviewer please help trigger a retry? Thanks. |
| Py_ssize_t length = sym_tuple_length(arg); | ||
| if (length >= 0) { | ||
| temp = PyLong_FromSsize_t(length); | ||
| if (temp == NULL) { | ||
| goto error; | ||
| } | ||
| } | ||
| else if (sym_is_const(ctx, arg)) { | ||
| PyObject *const_val = sym_get_const(ctx, arg); | ||
| if (const_val != NULL && PyUnicode_CheckExact(const_val)) { | ||
| temp = PyLong_FromSsize_t(PyUnicode_GET_LENGTH(const_val)); | ||
| if (temp == NULL) { | ||
| goto error; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we can optimize the logic here:
if (length < 0 && sym_is_const(ctx, arg)) {
PyObject *const_val = sym_get_const(ctx, arg);
if (const_val != NULL && PyUnicode_CheckExact(const_val)) {
length = PyUnicode_GET_LENGTH(const_val);
}
}
if (length >= 0) {
PyObject *temp = PyLong_FromSsize_t(length);
if (temp == NULL) {
goto error;
}
if (_Py_IsImmortal(temp)) {
ADD_OP(_SHUFFLE_3_LOAD_CONST_INLINE_BORROW,
0, (uintptr_t)temp);
}
res = sym_new_const(ctx, temp);
Py_DECREF(temp);
}
Python/optimizer_bytecodes.c
Outdated
| if (const_val != NULL && PyUnicode_CheckExact(const_val)) { | ||
| temp = PyLong_FromSsize_t(PyUnicode_GET_LENGTH(const_val)); |
There was a problem hiding this comment.
Can we also deal with the PyBytes_CheckExact + PyBytes_GET_SIZE?
|
Sorry I'm busy right now, and will review this over the weekend. Thanks for your patience. |
|
Pulling in main to re-trigger CI. |
|
Thanks for the contribution! |
|
Thanks @aisk @cocolato @Fidget-Spinner for the reviews |
4 participants
Uh oh!
There was an error while loading. Please reload this page.