bpo-42152: Optimize out uses of PyDict_GetItemWithError if its result is not used. - #22986
serhiy-storchaka merged 1 commit into
Conversation
Sorry, something went wrong.
… is not used. Use PyDict_Contains and PyDict_SetDefault if appropriate.
| int r = _PyDict_ContainsId(d, &PyId___builtins__); | ||
| if (r == 0) { | ||
| r = _PyDict_SetItemId(d, &PyId___builtins__, | ||
| PyEval_GetBuiltins()); |
There was a problem hiding this comment.
PyEval_GetBuiltins() is cheap function so we can use PyDict_SetDefault() here, and builtin_eval_impl, builtin_exec_impl if you want to.
Sorry, something went wrong.
There was a problem hiding this comment.
There is no _PyDict_SetDefaultId() and I do not want to introduce it just for these three cases. Using PyDict_SetDefault() + _PyUnicode_FromId() will not make large difference:
int r = _PyDict_ContainsId(d, &PyId___builtins__);
if (r == 0) {
r = _PyDict_SetItemId(d, &PyId___builtins__,
PyEval_GetBuiltins());
}
if (r < 0) {
remove_module(tstate, name);
return NULL;
}vs
PyObject *tmp = _PyUnicode_FromId(&PyId___builtins__);
if (tmp != NULL) {
tmp = PyDict_SetDefault(d, tmp, PyEval_GetBuiltins());
}
if (tmp == NULL) {
remove_module(tstate, name);
return NULL;
}
Sorry, something went wrong.
methane
left a comment
There was a problem hiding this comment.
Looks very nice.
Sorry, something went wrong.
|
LGTM. The PR title say this is an optimization, but I don't see that as relevant or justified. |
Sorry, something went wrong.
…ythonGH-22986) If PyDict_GetItemWithError is only used to check whether the key is in dict, it is better to use PyDict_Contains instead. And if it is used in combination with PyDict_SetItem, PyDict_SetDefault can replace the combination.
Use PyDict_Contains and PyDict_SetDefault if appropriate.
https://bugs.python.org/issue42152