This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Optimize LOAD_NAME bytecode
Type: performance Stage: patch review
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, python-dev, rhettinger, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2015-11-05 10:11 by vstinner, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
pydict_loadname.patch vstinner, 2015-11-05 10:11 review
pydict_loadname-2.patch vstinner, 2015-11-05 13:38 review
Messages (7)
msg254097 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-11-05 10:11
The LOAD_GLOBAL bytecode has a fast-path when globals and builtins have exactly the type 'dict'. It calls the _PyDict_LoadGlobal() function.

I propose to implement a similar optimization for LOAD_NAME, see attached patch.

The patch also fixes LOAD_GLOBAL and LOAD_NAME bytecodes when locals, globals or builtins are not exactly the type 'dict'. It clears the KeyError before trying the next PyObject_GetItem().

The patch changes also _PyDict_LoadGlobal() to call PyObject_Hash() if the hash was not computed yet. It might make it a little bit faster.
msg254101 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-05 11:26
When LOAD_NAME is generated? Is it worth to optimize this case? Aren't LOAD_FAST and LOAD_GLOBAL used in performance critical code?

It looks to me that there is a bug in fast path of _PyDict_LoadGlobal. If the first lookup fails, it can raise an exception. We have to add

    if (PyErr_Occurred())
        return NULL;

before the second lookup.

Just for reference, the fast path for LOAD_GLOBAL was added in 8f8fe990e82c.
msg254102 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-11-05 11:49
> When LOAD_NAME is generated? Is it worth to optimize this case? Aren't LOAD_FAST and LOAD_GLOBAL used in performance critical code?

I guess that it's only used to execute the body of modules.

> Is it worth to optimize this case?

Hum, I don't know :-)

> It looks to me that there is a bug in fast path of _PyDict_LoadGlobal. If the first lookup fails, it can raise an exception.

Sorry, where exactly? Can you maybe put a comment on the review? I see many checks to handle errors.
msg254112 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-11-05 13:38
Rebased patch (ceval.c was modified by the changeset of c1414f80ebc9, issue #25556).
msg254956 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-20 08:27
New changeset c35d65c9ded3 by Victor Stinner in branch 'default':
Issue #25557: Refactor _PyDict_LoadGlobal()
https://hg.python.org/cpython/rev/c35d65c9ded3
msg254957 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-11-20 08:28
Since LOAD_NAME is rare, I don't think that it's worth to optimize it.

I pushed a partial version of pydict_loadname-2.patch to add comments to the code.
msg254959 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-20 08:58
Thanks Victor.
History
Date User Action Args
2022-04-11 14:58:23adminsetgithub: 69743
2015-11-20 08:58:24serhiy.storchakasetmessages: + msg254959
2015-11-20 08:28:57vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg254957
2015-11-20 08:27:56python-devsetnosy: + python-dev
messages: + msg254956
2015-11-05 13:38:01vstinnersetfiles: + pydict_loadname-2.patch

messages: + msg254112
2015-11-05 11:49:17vstinnersetmessages: + msg254102
2015-11-05 11:26:13serhiy.storchakasetnosy: + rhettinger, benjamin.peterson
messages: + msg254101
2015-11-05 10:16:34serhiy.storchakasetstage: patch review
2015-11-05 10:11:19vstinnercreate