Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RAII - memory management #658

Closed
wants to merge 0 commits into from
Closed

Conversation

renehiemstra
Copy link

This pull request adds memory management to terra using the concept of RAII.

The design adds the following four metamethods to the language that enable the implementation of smart (shared) pointer types:

  1. __init(self : &A)
  2. __move(self : &A)
  3. __copy(self : &A)
  4. __dtor(self : &A)

If implemented, these methods are inserted judiciously during the type checking phase, implemented in terralub.lua. All these metamethods can be implemented as macro's or as terra functions.

The design does not introduce any breaking changes. No new keywords are introduced. Heap resources are acquired and released using the regular C stdlib functions such malloc and free.

I will explain the metamethods in some more depth:

  • __init is used to initialize pointer variables to nil, making them safe to delete. The implementation checks for an __init metamethod in any defvar statement: var a : A and applies it if it exists. If there is an initializer, var a : A = ... then the implementation does not search for an __init metamethod. A deferred call to __dtor is added in this case and a call to __copy if the right-hand-side involves a struct variable.
  • __dtor is used to free heap memory. A defer node to this metamethod is added during type-checking such that __dtor is called at the end of a scope. __dtor is also applied before any assignment, e.g. a = alloc(...), to free old resources.
  • __move is needed in order to return from a function. It moves resources to a temporary variable , and sets the old pointer variable to nil such that the deferred destructor call does not free the heap resources. __move will be called on every returned variable (that implements it) in a return statement.
  • __copy enables side effects in a statement var b = a or b = a, such as increasing a reference counter. It acts solely on the right-hand-side variable in the assignment expression.

I have added a test file smartptr.t in the tests folder. It contains an implementation of a smart shared pointer and two integration tests that test all four metamethods. These tests print out the control flow of the program to check when each meta method is invoked.

All existing tests in the tests folder pass on my machine (except for a few that did not pass prior to the changes)

If the design is accepted, there are a few things left to do:

  1. Additional testing in real use cases.
  2. Implement output for prettystring in terralib.lua.
  3. Adding a smart pointer implementation to std.t.
  4. Documentation of the feature on github and the terra website.

I am happy to work further on improving the design where needed.

@chriku
Copy link
Contributor

chriku commented May 11, 2024

I generally like the idea, but (at least) your __move returns an object, which is then copied by value by terra. If I read that correctly, this means stuff like offset_ptrs (container that saves the offset between this-ptr and the value, VERY useful for IPC stuff) are not possible, so maybe your move and copy could take a second (first?) parameter in which to move/copy the object into? Another example would be memory reuse in containers, as fully destroying the object and constructing a new one is far more costly than just reassigning the content, so an optimized move/copy assignment would be appreciated.

Also: I first thought, that you do not have a method for move assignment (of existing variables), but it's of course possible to make an assign method, so maybe elaborate in that direction to avoid confusion when skimming over the PR?

Also Also: Your RAII code does not do any memory allocation, only the not-shared_ptr test script. Maybe clarify on that topic, to avoid stirring up any memory management debates

Also Also Also: you commented out some sdkroot platform code, probably so it works on your machine, but this shouldn't be part of the PR

@renehiemstra
Copy link
Author

__move

  • I introduced the __move metamethod to return from a function and to get around the deferred destructor call to __dtor. I can investigate some ideas to do this in a better way, such that a user-defined __move method and an additional copy is not needed.
  • I will investigate what is needed to implement an offset_ptr type .
  • In Terra we cannot currently overload the @ operator. So if we want pointer semantics then we could think about enabling this?

__copy

  • Terra does not have rvalue references, so I don't see how we can have a specialized move-asignment and a specialized copy-assignment at the same time.
  • We could do something similar as in rust, where everything is moved by default and a copy trait enables specialized behavior (for instance a bit-copy). In Terra we would do the opposite (to avoid breaking compatibility), a bit-copy by default and a __copy(self : &A, other : &A) metamethod could be called to induce specialized behavior (you could implement a move assignment here). What do you think about this?

__init
It may be worthwhile to have a default __init that sets pointers to nil whenever a __dtor is implemented.

Also Also: The point of the contribution is to add some metamethods to enable RAII, without introducing breaking changes. This leaves memory allocation using the stdlib C library in the hands of the programmer.

Also Also Also: right, the commented out platform code is a separate issue that I forgot to undo.

@elliottslaughter
Copy link
Member

elliottslaughter commented May 15, 2024

As I said on Zulip, I appreciate the work that @renehiemstra has put into this and agree that something along these lines would be useful.

Unfortunately I am continuing to run out of time to actually look at this in depth.

I think it would be nice to make sure that this can capture basic patterns similar to std::vector, std::string, etc. I suppose at a superficial level those are similar to smart pointers, but one would generally expect (a) owning semantics, not shared semantics, and (b) copies are especially painful if we don't optimize those.

I have enabled the CI to run so that you can at least see those results. I may have to re-enable it on each push since this is a first-time contribution.

Edit: I also wanted to check that the metaprogramming APIs are compositional: i.e., you can create a vector(vector(int)) or a vector(string) or etc. and have it do the right thing. I'm not saying the current version isn't, but I just haven't had time to think through it.

@renehiemstra
Copy link
Author

renehiemstra commented May 16, 2024

I was able to implement a __dtor metamethod (without using defer) that is invoked at the end of each new scope, before a possible return-statement, and only acts on the variables that are not returned. As such, destruction is really tied to the lifetime of an object, which is precisely what we need. The __move metamethod and the additional copy are no longer needed.

I also implemented a __copy metamethod that takes two arguments

    A.metamethods.__copy(from : &A, to : &B)

and / or

    A.metamethods.__copy(from : &B, to : &A)

The arguments of __copy can be different and it can be an overloaded function.

It can also be used on object construction,

    var a : A
    a.data = ...
    var b = a

which is the same as writing

    var a : A
    a.data = ...
    var b : A
    b = a

The __init metamethod is called in object initialization, if it is implemented.

I think the above covers all the use cases of smart containers and pointers in c++ (e.g. std::vector, std::unique_ptr, and std::shared_ptr) and it should also work for the case of boost::offset_ptr

I still need to make the metamethods work properly in the case of compositional API's, like you mentioned (vector(vector(int)) or a vector(string)).

Also, I need to handle some corner cases. In Terra its allowed to combine a logical statement in a return statment, e.g.

    return some_bool_flag and x or y

I am not sure how to deal with these yet, because this is runtime information.

@renehiemstra
Copy link
Author

Reopened in #659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants