[devreal]

The normal object life-time using PARSEC_OBJ_NEW, PARSEC_OBJ_RETAIN, and PARSEC_OBJ_RELEASE currently relies exclusively on malloc/free. Any objects with custom allocation strategies cannot use PARSEC_OBJ_RETAIN and PARSEC_OBJ_RELEASE. This makes it hard to use free-lists with ref-counted objects.

Add a pointer to an allocator to the parsec_object_t, which is used to release the object. Add new PARSEC_OBJ_NEW* macros that allocate using a corresponding allocators (mempool, and a new simple freelist). This enables us to return objects to where they came from once their ref-count drops to zero.

Apply this to the device-side data copy objects by allocating them from a device-specific free-list. This mitigates excessive malloc/free calls in the device manager, which allocates and frees a potentially sizable number of copies for each task.

[Copilot]

🟡 Changes recommended

GPU allocator lifetime, Windows deallocation, and public API compatibility issues remain unresolved.

4 open findings

• Invalid free of Windows-aligned LIFO storage · New
• Free-list destroyed while outstanding copies still reference it · New
• Object mempool uses mismatched deallocator on Windows · New
• Public custom-release API removed without compatibility support · New

What changed in this PR

Makes PaRSEC objects allocator-aware, enabling reference-counted objects to return storage to mempools or free-lists.

Changes:

• Adds allocator-aware object APIs and a thread-safe object free-list.
• Integrates object construction/destruction with mempools.
• Recycles GPU data-copy objects and adds allocator tests.

File	Description
tests/​class/​Testings.cmake	Registers allocator tests.
tests/​class/​obj_allocator.c	Tests allocators, free-lists, and mempools.
tests/​class/​CMakeLists.txt	Builds allocator test variants.
parsec/​parsec.c	Uses raw mempool storage before task copying.
parsec/​parsec_internal.h	Associates copied tasks with their allocator.
parsec/​mempool.h	Exposes allocator-aware mempool APIs.
parsec/​mempool.c	Implements mempool allocator callbacks.
parsec/​mca/​device/​transfer_gpu.c	Updates GPU transfer-task recycling.
parsec/​mca/​device/​device_gpu.h	Adds the device-local copy free-list.
parsec/​mca/​device/​device_gpu.c	Recycles GPU data-copy objects.
parsec/​interfaces/​ptg/​ptg-compiler/​jdf2c.c	Removes redundant generated construction.
parsec/​interfaces/​dtd/​insert_function.c	Uses mempool construction for DTD tasks.
parsec/​data_dist/​matrix/​map_operator.c	Uses raw storage for copied tasks.
parsec/​CMakeLists.txt	Builds and installs the free-list API.
parsec/​class/​parsec_object.h	Defines allocator-aware object interfaces.
parsec/​class/​parsec_object.c	Implements default allocators.
parsec/​class/​parsec_obj_freelist.h	Declares object free-lists.
parsec/​class/​parsec_obj_freelist.c	Implements object free-lists.
parsec/​class/​list_item.h	Initializes raw storage for LIFO chaining.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[devreal]

This replaces the parsec_object_t::obj_release callback. The upside is that we now have access to the allocator state when releasing the object.

[devreal]

@bosilca Please take a look at this. I've been holding off on using it in my fork to avoid further divergence. Unless there is major opposition I will start integrating it though.

[bosilca]

The mechanism is right: one pointer in the object, resolved at release, no change to sizeof(parsec_object_t), and PARSEC_OBJ_NEW still goes straight to malloc. My comments are about the API shape and a few sharp edges.

Fold the allocator into PARSEC_OBJ_NEW rather than adding a parallel family

PARSEC_OBJ_NEW_WALLOCATOR(type, allocator) already is "PARSEC_OBJ_NEW plus an allocator argument", so I would rather see it spelled that way than given a new name. As it stands we get five macros where two would do, and the two convenience wrappers don't agree with each other on argument order:

PARSEC_OBJ_NEW_WALLOCATOR(type, allocator)            /* allocator last  */
PARSEC_OBJ_FREELIST_NEW(freelist, type)               /* allocator first */
PARSEC_THREAD_MEMPOOL_OBJ_NEW(type, thread_mempool)   /* allocator last  */

The last two expand to nothing but the first, so they add no capability and two opportunities to transpose the arguments. I would drop both, and PARSEC_OBJ_CONSTRUCT_WALLOCATOR likewise — parsec_object.h is already carrying a PARSEC_OBJ_CONSTRUCT_INTERNAL that its own comment labels "backwards compatible macro for mempool", and we shouldn't grow that debt.

A NULL allocator should give the old behaviour. NULL ought to be a legal value meaning "the default for this operation", resolved at construction time rather than at release: parsec_obj_new_wallocator(cls, NULL) stores &parsec_obj_malloc_allocator, and PARSEC_OBJ_CONSTRUCT_WALLOCATOR(obj, type, NULL) stores &parsec_obj_static_allocator. Resolving it there and not in parsec_obj_release_internal keeps the release path a single unconditional indirection and avoids having to guess, at release time, which of the two defaults a NULL field was supposed to mean. With that convention PARSEC_OBJ_NEW(type, NULL) is literally the old PARSEC_OBJ_NEW(type), and PARSEC_COPY_EXECUTION_CONTEXT can stop naming parsec_obj_static_allocator in an installed macro and just pass _mpool ? &_mpool->super : NULL.

That leaves the question of whether the argument should be mandatory. Making it mandatory is the most honest version but it is a flag day: roughly 84 call sites in 47 files in the runtime alone, plus DPLASMA and TTG, and it makes the overwhelmingly common case the verbose one. Argument-count dispatch so that both PARSEC_OBJ_NEW(type) and PARSEC_OBJ_NEW(type, allocator) work would avoid the churn, and we already depend on the GNU ##__VA_ARGS__ extension in utils/debug.h and interfaces/dtd/insert_function.h, so there is precedent; the costs are that the dispatch has to be written twice (the paranoid branch already wraps the call in parsec_obj_new_debug(...)), diagnostics at the call site get worse, and MSVC's traditional preprocessor mis-expands argument counting, which matters only insofar as we still care about the WIN64 path the LIFO code carries.

For completeness, the option that sounds most natural — putting the allocator in the class descriptor, PARSEC_OBJ_CLASS_INSTANCE(type, parent, ctor, dtor, allocator) — cannot express these use cases. The device data-copy free-list is per-device and a thread-mempool is per-thread, so one class must draw from many allocators at once: parsec_data_copy_t comes from plain malloc in three places in data.c and from N per-device free-lists in the same run. The allocator is a property of the call site, and thereafter of the object, not of the class. It might still be worth having as a per-class default later, but that is an addition rather than a replacement.

Fail loudly instead of warning once the free-list is gone

device_gpu.c:1140. I agree with your answer to the bot that no copies should be alive at device teardown, but the code is written as though they might be: it destructs the free-list and then warns and continues. After parsec_obj_freelist_destruct the lifo is NULL while obj_alloc/obj_release still point into the free-list, so a late PARSEC_DATA_COPY_RELEASE is a NULL dereference inside parsec_lifo_push, several frames away from the actual mistake. Two cheap lines make the invariant self-enforcing: assert(NULL != freelist->lifo) at the top of parsec_obj_freelist_alloc and parsec_obj_freelist_release, and parsec_fatal rather than parsec_warning when nb_in_use != 0, matching what parsec_device_memory_release already does one block earlier for a device that is still being managed.

The new parsec_mempool_construct assert is narrower than the constraint it describes

mempool.c:62. The back pointer is clobbered by LIFO chaining in every pool, not only in the ones with an obj_class, because parsec_lifo_push writes list_next whatever the element is. So the NULL == obj_class || exemption doesn't correspond to a real difference.

It isn't gratuitous, though: vp->dependencies_mempool would trip the assert if the exemption were dropped. In a release build offsetof(parsec_hashable_dependency_t, mempool_owner) is 24 and offsetof(parsec_list_item_t, list_next) is also 24, so pushing one of those elements back into the pool overwrites its own back pointer. That pool only works because parsec_release_local_OUT_dependencies re-assigns hd->mempool_owner after every allocation (parsec.c:1613). Pre-existing and out of scope here, but the comment above the assert currently reads as though the constraint were about object construction, and it would be worth saying that the real reason for the exemption is one pool that re-initializes its own back pointer every time.

parsec_thread_mempool_free now silently ignores the reference count

mempool.h:220. For a pool with an obj_class it destructs and recycles regardless of the count, while PARSEC_OBJ_RELEASE on the same object honours it. The two now look interchangeable in a way they didn't before, and mixing them on one object recycles the storage twice. All three DTD call sites already assert(obj_reference_count == 1) immediately before calling it, so promoting that assert into the obj_class branch of parsec_thread_mempool_free costs nothing and documents the contract where it belongs. Longer term this PR is what finally lets DTD drop its hand-rolled parsec_atomic_fetch_dec_int32 dance in favour of plain PARSEC_OBJ_RELEASE, which is worth mentioning in the commit message as the follow-up it enables.

parsec_thread_mempool_allocate became a function-like macro in an installed header

It loses its prototype, and any downstream use in a non-call position — taking its address, a C++ wrapper, a function-pointer table — stops compiling. Since the only thing the macro buys is __FILE__/__LINE__ capture for paranoid builds, define it only under #if defined(PARSEC_DEBUG_PARANOID) and keep a plain static inline function otherwise.

The removed public surface deserves a release note

parsec_release_t, parsec_obj_destruct_and_free, PARSEC_OBJ_CONSTRUCT_WRELEASE, parsec_object_t::obj_release and parsec_release_device_task_function_t/release_device_task all disappear from installed headers, and parsec_object_t's layout changes. You confirmed DPLASMA doesn't use PARSEC_OBJ_CONSTRUCT_WRELEASE; release_device_task is the one I would double-check against TTG, since that callback existed precisely so a DSL could own its wrappers, and the replacement (interposing an allocator) is a different shape of code. Note also that the callback used to fire unconditionally whereas PARSEC_OBJ_RELEASE only reaches the allocator when the count hits zero — irrelevant today, since nothing retains a wrapper, but it is a semantic difference.

Worth calling out in the commit messages

parsec_thread_mempool_free(task->mempool_owner, task) in transfer_gpu.c:380 is a real bug fix rather than a cleanup: the d2h task was being pushed onto whichever thread's pool happened to be running the manager. It is also load-bearing now, since the new paranoid owner assert in parsec_thread_mempool_push would fire on the old code. Same for the (char*)&(__pseudo_elt.##field_name) to .field_name fix, and for the resurrection of the owner assert in parsec_thread_mempool_free, which referenced undefined variables and so could never have compiled under PARSEC_DEBUG_ENABLE.

Nits

• parsec_obj_freelist_construct doesn't check PARSEC_OBJ_NEW(parsec_lifo_t) for NULL, and every later use dereferences it.
• parsec_obj_freelist.h documents its group as @ingroup parsec_internal_classes but the header is installed as public.
• tests/class/obj_allocator.c uses __sync_fetch_and_add directly where the sibling class tests use parsec_atomic_fetch_inc_int32.
• counted_t::payload[200] could use a one-line comment saying it exists to push the class past sizeof(parsec_list_item_t).

[devreal]

Thanks for the thorough review, @bosilca. Addressed in bcea8e5:

Allocator folded into PARSEC_OBJ_NEW / PARSEC_OBJ_CONSTRUCT. Both take the allocator as an optional last argument, dispatched on argument count: PARSEC_OBJ_NEW(type) / PARSEC_OBJ_NEW(type, allocator) and PARSEC_OBJ_CONSTRUCT(obj, type) / PARSEC_OBJ_CONSTRUCT(obj, type, allocator), so none of the existing call sites change. The dispatch uses plain C99 __VA_ARGS__ (no ##), with the usual EXPAND indirection for MSVC's traditional preprocessor (untested there). It is written once; the paranoid file/line capture is a separate wrapper shared by both forms. NULL selects the default of each operation and is resolved at construction, so parsec_obj_release_internal stays a single unconditional indirection. PARSEC_OBJ_NEW_WALLOCATOR, PARSEC_OBJ_CONSTRUCT_WALLOCATOR, PARSEC_OBJ_FREELIST_NEW, PARSEC_THREAD_MEMPOOL_OBJ_NEW and the old PARSEC_OBJ_CONSTRUCT_INTERNAL are gone, and PARSEC_COPY_EXECUTION_CONTEXT now passes _mpool ? &_mpool->super : NULL. Agreed on a per-class default allocator: possible later as an addition, not a replacement.

Free-list lifetime. The free-list asserts it is still alive on allocation and release, and device teardown with outstanding copies is now parsec_fatal. parsec_obj_freelist_construct returns PARSEC_ERR_OUT_OF_RESOURCE if the LIFO cannot be created, and the device checks it.

parsec_mempool_construct assert. The comment now states the real constraint (LIFO chaining overwrites the list item of every element) and that pools without a class are exempt only because the dependencies mempool re-assigns its back pointer after every allocation.

parsec_thread_mempool_free. Asserts a reference count of 1 for pools with a class, and its documentation says not to mix it with PARSEC_OBJ_RELEASE on the same object. The DTD cleanup to plain PARSEC_OBJ_RELEASE is noted in the commit message as the follow-up this enables.

parsec_thread_mempool_allocate. A static inline function again; the macro exists only under PARSEC_DEBUG_PARANOID.

Release note. CHANGELOG entries for the new API, the parsec_object_t layout change, the mempool construct/destruct change, everything removed (including release_device_task, with the note that the allocator is only reached at a zero count, unlike the callback), and the fixes. I checked TTG: it never sets release_device_task. It builds its wrapper with calloc + PARSEC_OBJ_CONSTRUCT(gpu_task, parsec_list_item_t) and still uses the old flow[] field, so it already trails the current GPU API; when it catches up it should pass &parsec_obj_malloc_allocator to PARSEC_OBJ_CONSTRUCT so the calloc'd memory is freed.

Commit message. The d2h owner fix, the .##field_name fix and the resurrected owner assert are called out as fixes.

Nits. The test uses parsec_atomic_fetch_inc_int32 and payload has its comment; the test also covers the NULL-allocator forms. I left @ingroup parsec_internal_classes as is: every installed class header, including parsec_object.h and lifo.h, uses that group, so renaming it is a doc-wide change better done separately.

Tested with Release and paranoid builds on macOS (no GPU here): 90/90 shared-memory tests pass in paranoid; in Release, runtime/scheduling:ltq segfaults in __data_repo_entry_used_once, which reproduces identically on master. The GPU sources and dtd_test_cuda_again_async.c were only compile-checked. Both macro forms also compile as C99 -pedantic and as C++17.

[bosilca]

Love it !