Skip to content

Conversation

@bosilca
Copy link
Contributor

@bosilca bosilca commented Feb 4, 2025

The new destructor need to return a continuation value, 0 if the chain of destructors will continue to be called, 1 if no other destructors shall be called on this object. As a result, we can now recycle parsec_objects for as long as they are correctly initialized twice. The first time, when the object is stored in some internal cache (in order to be able to properly chain it) and the second time before returning a newly allocated object back to the user.

Fixes #724.

Per @devreal suggestion here is a use case:

struct derived_t { lifo_elem_t super; ... }; 
ALLOC(obj)
  -> [
       -> CONSTRUCT(obj, derived_t)
       -> USE(obj)
       -> RELEASE(obj)
         // inside the destructor of derived_t
         -> no CONSTRUCT(obj, lifo_elem_t) is necessary as the object is already derived from lifo_elem_t, and the destructor of the lifo_elem_t has not yet been called (they are called in reverse class order).
         -> PUSH(lifo, obj) // stash away
         // destructor of derived_t returns 1
       -> POP(lifo, obj)  // get it back
     ]* // rinse and repeat
-> FREE(obj) // finally release the memory

The new destructor need to return a continuation value, 0 if the chain
of destructors will contiune to be called, 1 if no other destructors
shall be called on this object. As a result, we can now recycle
parsec_objects for as long as they are correcty reinitialized twice. The
first time, when the object is stored in some internal cache (in order
to be able to properly chain it) and the second time before returning a
newly allocated object back to the user.

Signed-off-by: George Bosilca <[email protected]>
@bosilca bosilca requested a review from a team as a code owner February 4, 2025 07:13
Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good solution 👍 Little worried about changing the ABI but I guess TTG is the only external user we know?

Also, just to document this: the life-cycle of an object would look like this:

struct derived_t { lifo_elem_t super; ... }; 
ALLOC(obj)
  -> [
       -> CONSTRUCT(obj, derived_t)
       -> USE(obj)
       -> RELEASE(obj)
         // inside the destructor of derived_t
         -> CONSTRUCT(obj, lifo_elem_t)
         -> PUSH(lifo, obj) // stash away
         // destructor of derived_t returns 1
       -> POP(lifo, obj)  // get it back
       -> DESTRUCT(obj)   // destruct the lifo_elem_t
     ]* // rinse and repeat
-> FREE(obj) // finally release the memory

It's a bit strange to construct an object in memory where the previous object was not destroyed (i.e., the lifo_elem_t was never released and we're overwriting it). I guess that is fine as long as the base class does not hold any resources that must be released before reconstructing it.

@bosilca bosilca marked this pull request as draft February 4, 2025 15:50
@bosilca
Copy link
Contributor Author

bosilca commented Feb 4, 2025

We need to think about this a little longer, as the solution proposed here does not allow the object to be forcefully released. The objects could implement their own logic in the destructor that will always release the object if we are in some final state (aka. during parsec_fini), but this is error prone, and does not account for calls to OBJ_DESTRUCT.

@devreal
Copy link
Contributor

devreal commented Feb 4, 2025

-> no CONSTRUCT(obj, lifo_elem_t) is necessary as the object is already derived from lifo_elem_t, and the destructor of the lifo_elem_t has not yet been called (they are called in reverse class order).

The reason I had the extra CONSTRUCT in my example is because here we reset the magic object ID to 0, so I assume the object is technically not in a valid state anymore (i.e., we cannot call PARSEC_OBJ_DESTRUCT on the rest of the object because it will trip an assert).

@bosilca
Copy link
Contributor Author

bosilca commented Feb 4, 2025

Correct, the object is not in a correct state and shall not be released unless (re)constructed. But is does not have to be constructed in order to be added to the lifo, because the magic is only tested during release.

@bosilca
Copy link
Contributor Author

bosilca commented Feb 5, 2025

All these approaches have the same underlying problem: they have no clear way to release the objects (not put them back into a list but really free them).

@abouteiller
Copy link
Contributor

We did #731 instead. This one has some extra features but we don't use them atm.

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.

Release function for parsec_object_t

3 participants