bpo-44945: Specialize BINARY_ADD #27967
Conversation
markshannon
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @markshannon for commit a22af9a 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
| } | ||
| if (left_type == &PyUnicode_Type) { | ||
| int next_opcode = _Py_OPCODE(instr[1]); | ||
| if (next_opcode == STORE_FAST) { |
There was a problem hiding this comment.
I'm surprised this check alone is enough, seems pretty easy to trick it with something like s = y + ''. Then again the runtime check DEOPT_IF(var != left, BINARY_ADD); will catch that.
| @@ -121,7 +121,8 @@ _Py_GetSpecializationStats(void) { | |||
| int err = 0; | |||
| err += add_stat_dict(stats, LOAD_ATTR, "load_attr"); | |||
| err += add_stat_dict(stats, LOAD_GLOBAL, "load_global"); | |||
| err += add_stat_dict(stats, LOAD_GLOBAL, "load_method"); | |||
| err += add_stat_dict(stats, LOAD_METHOD, "load_method"); | |||
There was a problem hiding this comment.
Woops, thanks for fixing this.
markshannon
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @markshannon for commit 3146f48 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
| STAT_INC(BINARY_ADD, hit); | ||
| record_hit_inline(next_instr, oparg); | ||
| GETLOCAL(next_oparg) = NULL; | ||
| Py_DECREF(left); |
There was a problem hiding this comment.
Feel free to ignore, but you can reorder this to
PyUnicode_Append(&SECOND(), right);
STACK_SHRINK(1);
Py_DECREF(right);
Py_DECREF(left);
if (TOP() == NULL) {
goto error;
}
Then if you'd like, you can factor out the code from STACK_SHRINK(1); onwards into a BINARY_ADD_FOOTER macro, and reuse that for all BINARY_ADD_* instructions.
There was a problem hiding this comment.
I'm inclined to avoid using macros.
IMO, macros only help readability if they are function like, and self descriptive. E.g. Py_DECREF.
|
Failure on buildbot/AMD64 Arch Linux Asan Debug PR is a persistent failure unrelated to this PR. |
Adds the following specializations:
BINARY_ADD_UNICODE_INPLACE_FASTis needed to keepfor ...: s = s + ...O(ln(n)), rather than O(n**2).About 1% faster
https://bugs.python.org/issue44945