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: Generate LOAD_ATTR+CALL_FUNCTION instead of LOAD_METHOD+CALL_METHOD for imports
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: BTaskaya Nosy List: BTaskaya, Mark.Shannon, corona10
Priority: normal Keywords: patch

Created on 2021-06-04 16:33 by BTaskaya, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26677 merged BTaskaya, 2021-06-11 17:43
PR 26983 merged BTaskaya, 2021-07-01 19:00
Messages (5)
msg395099 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2021-06-04 16:33
import foo
def func():
    return foo.bar()

The snippet above will generate the following code;

  2           0 LOAD_GLOBAL              0 (foo)
              2 LOAD_METHOD              1 (bar)
              4 CALL_METHOD              0
              6 RETURN_VALUE

Though this will make things harder for specializing the LOAD_ATTR for modules since now the handling of LOAD_METHOD for that case is necessary so for the imports that we can infer during the symbol analysis pass, we'll generate LOAD_ATTR+CALL_ATTR instead of LOAD_METHOD+CALL_METHOD and hopefully the generated code will get specialized via the PEP 659.

Ref: https://github.com/faster-cpython/ideas/issues/55#issuecomment-853101039
msg395103 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2021-06-04 16:40
@mark.shannon what do you think about doing this both for `import <module>` and `from <module> import <something>`. It will definitely simplify the implementation (no need to extra visitors or flags, just using the default DEF_IMPORT one https://github.com/isidentical/cpython/commit/f8f8fcee4d1480970c356eec0f23c326b9fe674d) and also might help you to specialize other cases too (e.g importing submodules, from concurrent import futures).
msg395516 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-06-10 08:37
Yes. Simpler is good.


I think it will also be better for performance:

In general, we don't know what X is in `from Y import X`. It could be a module or anything else.

However, if we are accessing an attribute it is quite likely to be a module or class.
For `X` defined by `from Y import X`, `X` is likely to be a module, class, function, or some sort of constant like a string, int or Enum.

If it is a string, int or function then it is rare to call a method on it, so we can ignore that case.
Calling methods on an Enum constant is probably not very common either (I'm guessing here)

For a module, `LOAD_ATTR; CALL_FUNCTION` is clearly better than `LOAD_METHOD; CALL_METHOD`.

For a class, specializing `LOAD_ATTR` is no more complex than `LOAD_METHOD`, probably simpler.
So, for a class `LOAD_ATTR; CALL_FUNCTION` is no worse than `LOAD_METHOD; CALL_METHOD`, and might be better.


Overall, it looks like `X.foo()` when `X` is defiend by `from Y import X` is best as `LOAD_ATTR; CALL_FUNCTION` not `LOAD_METHOD; CALL_METHOD`.
msg396802 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-06-30 22:53
New changeset 1b28187a0e3e914ee48de8032cbba0a965dd5563 by Batuhan Taskaya in branch 'main':
bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported objects (GH-26677)
https://github.com/python/cpython/commit/1b28187a0e3e914ee48de8032cbba0a965dd5563
msg396826 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2021-07-01 19:25
New changeset 0d7f61ddb074659d8c18c8f5ac86a6a18e41f9e5 by Batuhan Taskaya in branch 'main':
bpo-44313: bump up magic (#26983)
https://github.com/python/cpython/commit/0d7f61ddb074659d8c18c8f5ac86a6a18e41f9e5
History
Date User Action Args
2022-04-11 14:59:46adminsetgithub: 88479
2021-07-01 19:25:38BTaskayasetmessages: + msg396826
2021-07-01 19:00:34BTaskayasetpull_requests: + pull_request25545
2021-07-01 07:53:42BTaskayasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-06-30 22:53:59Mark.Shannonsetmessages: + msg396802
2021-06-11 17:43:04BTaskayasetkeywords: + patch
stage: patch review
pull_requests: + pull_request25263
2021-06-10 09:31:51corona10setnosy: + corona10
2021-06-10 08:37:15Mark.Shannonsetmessages: + msg395516
2021-06-04 16:40:59BTaskayasetmessages: + msg395103
2021-06-04 16:33:18BTaskayacreate