REPL leaks memory #386
Comments
Partly (very partly!) addresses issue #386. Most importantly, at the REPL command line, each invocation does not now lead to increased memory usage (unless you define a function/lambda).
|
Yes, it needs a proper solution. Something like you suggest, with a table of pointers per module, and then let the garbage collector do the work of freeing inaccessible bytecode. I did a quick, partial fix. |
Attempt to address issue #386. unique_code_id's have been removed and replaced with a pointer to the "raw code" information. This pointer is stored in the actual byte code (aligned, so the GC can trace it), so that raw code (ie byte code, native code and inline assembler) is kept only for as long as it is needed. In memory it's now like a tree: the outer module's byte code points directly to its children's raw code. So when the outer code gets freed, if there are no remaining functions that need the raw code, then the children's code gets freed as well. This is pretty much like CPython does it, except that CPython stores indexes in the byte code rather than machine pointers. These indices index the per-function constant table in order to find the relevant code.
|
@pfalcon What do you think of the "proper" fix to this? |
|
Thanks for going for this! Yes, apparently storing physical pointer is only a temporary solution. But I now see that there's extra complexity which I missed: need to store asm/viper code too. Anyway, I don't have other ideas except sounded before. The most efficient storage format would be to embed bodies of functions directly into current bytecode stream (like MAKE_FUNCTION takes an int meaning that next N bytes are bytecode chunk, wrap it in structure and put on TOS). Best storage, but impossible to generate it such, so would need to restructure after compile, which is waste of mem. So apparently just have per-module "code_id" tables, that will allow individual bytecode chunks live in own memory blocks, but make lookup and persistence easy. Per-module because that's the unit of persistence. But then it would mean that in REPL, each expression will be either in own module, which is not correct, or we get the subj problem back (likely would need to null ptr to last bytecode object explicitly). So, dunno about details, but #222 should really be cornerstone of any future work on this. |
|
I don't think it's the right approach (logically) to embed code within code. I think you are wrong that "per-module is the unit of persistence". The REPL case is a good example: function definitions can "leak" out of the REPL module and persist, even when you no longer need tho parent of that function. Same with module (I guess): you can capture a function from a module (eg a lambda), then delete that module, but still have a reference to the captured function. |
|
By persistence I mean saving/loading bytecode from external storage. CPy creates .pyc per module, I don't think we should aim for other arrangement.
And the function will have reference to its defining global scope. There would be a question does the global scope has reference to its module object to prevent its GC, but it's moot one, because Python doesn't support unloading modules - all modules ever loaded added to sys.modules and thus kept alive. So, it's only REPL somewhat special (and its expressions should be evaluated within one module's context for sure), but as long as we don't have collection with pointers to all bytecodes accumulated, we should be fine automagically - we get pointer from compiler, then run it, and then ready to displace that pointer. If the bytecode was defining a function, its reference would have been stored in global scope, so will be alive. And if REPL statement didn't define anything, it can be GCed. |
|
Good point about lambda having reference to its global scope (ie module scope), but I still think there is no reference to the modules outer code block. |
|
Yeah, but module's outer code block contains only on-load init code - in basic case, just assignment of bytecode objects to function symbols. And this code block indeed executes only on initial load, not for example on reimporting module into new namespace. So, it indeed could be GCed. It wouldn't be if all code objects are referenced by module's local indirection table; then we actually could null it out as optimization. But for precompiled bytecode, it's still a moot point, as we'd likely load entire module into one memory chunk. |
Try it on stm or unix port - but you'll see that as you evaluate each new expression, memory usage grows, and running GC explicitly doesn't help. That's because of unique_codes which forever holds references to any bytecode object compiled. It'c clear that unique_codes is used for 2 purposes: 1) convert a raw pointer (which cannot be represented as mp_obj_t) into something representable as mp_obj_t (SMALL_INT specifically); 2) to make sure a particular code is not garbage collected between time it is compiled and time it is assigned to some variable. Thinking about it, 2) is the most grave reason for its existance, but it does its work too well - up to not releasing any code object all, even if it's not referenced by any value any longer.
I don't know how exactly to resolve this issue, but unique_codes in it's current shape of global ever-growing array should go. One "obvious" way to resolve that would be to exploit property that in the given point of execution flow, it's always known to which variable the most recent code object should be assigned. But it's unclear what byte code structure that would require - something like recursive bytecode streams with bytecode streams. But that would be pretty big departure from CPython model.
Another workaround (sic) would be to have module-scope unique_codes during compiler, and null entries which get assigned to global vars (but not others - that's a bit complicated rules, why I call it workaround).
Finally - the way closest to how CPython does it, is to have code objects belong to a module code container, not stand on its own (this will ensure protection against spurious GC). Within module container, codes reference each other by byte offsets, not separate ID->(offset|ptr) mapping (such mapping will be of course needed during compile, but at the end of it, code objects can be "linked" and such mapping removed). This last choice will be the great step towards #222.
The text was updated successfully, but these errors were encountered: