faster-cpython / ideas

1.67k stars 49 forks source link

Compact version of `LOAD_ATTR` #583

Open markshannon opened 1 year ago

markshannon commented 1 year ago

[EDIT: See below why there is little or no value in a compact form of LOAD_GLOBAL]

Module and class level code is "run once". It can, in theory be run many times, but it is generally run only once. The existence of caches for this code is quite wasteful, in terms of space.

LOAD_GLOBAL takes 10 bytes, and LOAD_ATTR takes 20.

We should add LOAD_GLOBAL_COMPACT and LOAD_ATTR_COMPACT bytecodes, which would behave like LOAD_GLOBAL and LOAD_ATTR but without counters or cache, so would only take 2 bytes each. The compiler can easily tell whether code is a "run once" context and emit the correct instruction.

With PEP 649 on the horizon, there will be even more "run once" code in the form of annotation closures and code objects.

JelleZijlstra commented 1 year ago

Note that module and class-level code generally uses LOAD_NAME, not LOAD_GLOBAL. LOAD_GLOBAL only appears within functions and if you use the global statement. You can technically do that in module and class-level code, but I don't see why you would.

markshannon commented 1 year ago

Module level variables are in the global scope and should use LOAD_GLOBAL. Why do they use LOAD_NAME?

JelleZijlstra commented 1 year ago

Just to be clear (you probably know this), LOAD_GLOBAL looks in globals, then builtins, while LOAD_NAME looks in locals, then globals, then builtins. During module execution, locals and globals are the same, so LOAD_NAME indeed seems redundant. However, module scopes are also used for exec(), and there the globals and locals can be different. So if we want modules to use LOAD_GLOBAL, the symtable/compiler have to be able to distinguish between real modules and exec.

I actually tried this recently because I also thought modules should be able to just use LOAD_GLOBAL, and when I ran the test suite a bunch of tests that use exec() failed.

markshannon commented 1 year ago

OK, so just LOAD_ATTR_COMPACT then.