Release function for parsec_object_t

[devreal]

Description

With #694 the GPU task structure becomes a proper PaRSEC object and carefully avoid calling PARSEC_OBJ_RELEASE on it to avoid freeing it explicitly and instead add a callback to free it. The regular task structure is already an object but we don't really utilize the object management macros because we want to utilize free lists, so we have a dedicated callback for its release. Data copies have to be heap allocated because they are freed so we cannot use free lists for them.

Describe the solution you'd like

Instead of calling free in PARSEC_OBJ_RELEASE we should unify the above approaches and add a release callback to the parsec_object_t that allows the entity allocating it to determine the way it should be freed. Then we can PARSEC_OBJ_RELEASE the object and the object will go back where it came from, e.g., a free-list. This avoids the careful dance we do with parsec_task_t and parsec_gpu_task_t and would allow the use of free lists for parsec_data_copy_t.

Describe alternatives you've considered

The current approach of type-specific deallocators is fragile and does not cover all objects that could benefit from non-standard allocation methods.

[devreal]

We have two proposals:

• Rework the parsec_object_t system. #726 : Changes the destruct callback to allow a break in the destruction sequence. Downside is that we enable possibly half-destroyed objects.
• Introduce managed objects #728 : adds a new base class that takes a release callback. Downside is that base objects like parsec_list_item_t will grow by 8B.

Here is another proposal:

• Add a release callback to the class_t that can be set via PARSEC_OBJ_CLASS_INSTANCE (in addition to the constructor and destructor) and defaults to free. That callback could stash the object instead of having it free'd. If object-specific behavior of release is needed it is the user's responsibility to store a release callback pointer in their derived object and have the class callback invoke that. We would use the callback from the most derived class of the released object.

[bosilca]

All these approaches have a common issue, making impossible to properly release an object at the end of the execution (when it is really supposed to be freed). We could imagine convoluted solutions, but nothing clean and straightforward.

At this point the cleanest approach would be #726 with an extra flag to the destructor. RETAIN/RELEASE could be used during the lifetime of the object (including when we expect it to be saved into some memory management structure), and DESTRUCT will be used to unconditionally release the object at any moment. This approach would solve the free problem but will break the existing relationship between CONSTRUCT/DESTRUCT and NEW/RETAIN/RELEASE as DESTRUCT is now required to be called on a NEW recycled object.

Unfortunately, this does not solve the issue because DESTRUCT does not free the object. So back to square 1.

[devreal]

No, releasing the memory is not an issue. If I tell you that I will handle the release of the object I will handle the release of the object. Not PaRSEC's business what I do with it. If I take it from a mempool I will put it back there and release the memory of the mempool at the end of the run. If the object is part of another object (like the GPU task in TTG) then I don't have to do anything, and we can still use PARSEC_OBJ_RELEASE on it.

#726 is not good because it leaves objects in some undefined state and it causes a huge API break. I realize that #728 suffers from the same problem.

#729 is the cleanest solution as it does not break any existing code (apart from the ABI break of the class_t), does not increase the size of any objects but the class and all objects stay in well-defined states.

[bosilca]

Correctly releasing the memory is an issue because calling free on your object does not clean all the inherited classes. If you think of a simple, one-level, inheritance of a base object class then you are correct, but that's not a real solution.

There is no undefined state in #726. If you are referring to the magic value, it is easy to just move its reset in the same branch as the free, and the object is always in a defined state.

#729 has the same issue, in addition to only solving half of the problem (the release part but without taking care of the symmetry with the allocation).

[devreal]

Correctly releasing the memory is an issue because calling free on your object does not clean all the inherited classes.

The object will be freed once all destructors have been called. No issue here.

#729 has the same issue, in addition to only solving half of the problem (the release part but without taking care of the symmetry with the allocation).

Allocation is not a problem. PaRSEC itself will never allocate a user-defined object. We're only concerned about application-provided objects here and the application will not use PARSEC_OBJ_NEW to allocate an element from a memory pool. We could add an allocation callback to the class, but I'm not sure what the value is.

[devreal]

There is no undefined state in #726. If you are referring to the magic value, it is easy to just move its reset in the same branch as the free, and the object is always in a defined state.

Some more thoughts:

• Assuming I stopped the destructor chain in my destruction callback, I stashed it away, and now I want to reconstruct it to use it again. If the base class holds resources, how will it know that it still holds them? We cannot assume that the uninitialized state is zero and we cannot communicate that part of the object is still alive.
• Similar problem when finally destroying the object: at the end I take the object out of my stash and want to discard it. Now I cannot call PARSEC_OBJ_DESTRUCT because half the object is already destroyed. There is no way for me to control the destruction of the rest of the object.

Today, we talked about the issue @abouteiller had in #623 (IIRC) that some objects are allocated dynamically while others are allocated on the stack. Maybe the right way is to add a callback to the parsec_object_t that allows us to control the release process entirely. If an object has a non-NULL release callback then PARSEC_OBJ_RELEASE calls that instead of the destructor chain and free. It's entirely left to the release callback to destruct the object and release it to where it came from (or reuse it). That way we can keep the object in a valid state while stashing it.

Also, we should not call free on any object unless it has been allocated by PARSEC_OBJ_NEW. That can be encoded in the object's release callback by having PARSEC_OBJ_NEW set a release callback that runs the destructors and calls free. Otherwise the default destructor is to just call the destructors. The application can overwrite the callback at any time and take full responsibility for the object's cleanup.