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.

Sign up for GitHub

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

Jump to bottom

Add a function to pre-set infos for all streams/devices/... #665

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add a function to pre-set infos for all streams/devices/... #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
956b2ab
Add a function to pre-set infos for all streams/devices/...
therault Aug 7, 2024
8279fbd
In multi-GPU runs, we need to set the target GPU before calling the u…
therault Aug 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion parsec/class/info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ parsec_info_id_t parsec_info_unregister(parsec_info_t *nfo, parsec_info_id_t iid
item2 = PARSEC_LIST_ITERATOR_NEXT(item2)) {
ioa = (parsec_info_object_array_t*)item2;
if(iid < ioa->known_infos && NULL != ioa->info_objects[iid]) {
if(NULL != ioa->ctx_set) ioa->ctx_set(ioa->ctx_set_obj);
ie->destructor(ioa->info_objects[iid], ie->des_data);
ioa->info_objects[iid] = NULL;
}
Expand Down Expand Up @@ -210,7 +211,7 @@ static void parsec_info_object_array_constructor(parsec_object_t *obj)
/* The constructor cannot set the info, as it does not take additional
* parameters. Thus, it is needed to call init after constructing the
* info_object_array. */
void parsec_info_object_array_init(parsec_info_object_array_t *oa, parsec_info_t *nfo, void *cons_obj)
void parsec_info_object_array_init(parsec_info_object_array_t *oa, parsec_info_t *nfo, void *cons_obj, parsec_info_set_ctx_fn ctx_set, void *ctx_set_param)
{
oa->known_infos = nfo->max_id+1;
parsec_list_push_front(&nfo->ioa_list, &oa->list_item);
Expand All @@ -220,6 +221,8 @@ void parsec_info_object_array_init(parsec_info_object_array_t *oa, parsec_info_t
oa->info_objects = calloc(sizeof(void*), oa->known_infos);
oa->infos = nfo;
oa->cons_obj = cons_obj;
oa->ctx_set = ctx_set;
oa->ctx_set_obj = ctx_set_param;
}

static void parsec_info_object_array_destructor(parsec_object_t *obj)
Expand Down Expand Up @@ -311,10 +314,26 @@ void *parsec_info_get(parsec_info_object_array_t *oa, parsec_info_id_t iid)
ie = parsec_info_lookup_by_iid(oa->infos, iid);
if(NULL == ie->constructor)
return ret;
if(NULL != oa->ctx_set) oa->ctx_set(oa->ctx_set_obj);
nio = ie->constructor(oa->cons_obj, ie->cons_data);
ret = parsec_info_test_and_set(oa, iid, nio, NULL);
if(ret != nio && NULL != ie->destructor) {
ie->destructor(nio, ie->des_data);
}
return ret;
}

void parsec_info_set_all(parsec_info_t *nfo, parsec_info_id_t iid)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest a different name:

Suggested change
void parsec_info_set_all(parsec_info_t *nfo, parsec_info_id_t iid)
void parsec_info_populate(parsec_info_t *nfo, parsec_info_id_t iid)

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me question the need for this function. It is only use in a single location, and based on what it is doing I don't see the need for a capability to populate the entire info set. Second issue with this populate capability, is that all memory allocation will be done in the context of the caller of the populate, instead of allowing the allocated objects to belong to the scope of the thread/stream that would use them.

In fact my main issue is with the entire info API. Because parsec_info_set nicely allows you to set a value for a key, but then parsec_info_get will magically allocate one for you even if you never set one. And there is no way to distinguish between the case where the value was properly set and the case where it was auto-generated.

{
parsec_list_item_t *li;
parsec_info_object_array_t *oa;

parsec_list_lock(&nfo->ioa_list);
for(li = PARSEC_LIST_ITERATOR_FIRST(&nfo->ioa_list);
li != PARSEC_LIST_ITERATOR_END(&nfo->ioa_list);
li = PARSEC_LIST_ITERATOR_NEXT(li)) {
oa = (parsec_info_object_array_t*)li;
parsec_info_get(oa, iid);
}
parsec_list_unlock(&nfo->ioa_list);
}
38 changes: 35 additions & 3 deletions parsec/class/info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ typedef void (*parsec_info_destructor_t)(void *elt, void *cb_data);
*/
typedef void *(*parsec_info_constructor_t)(void *obj, void *cb_data);

/**
* @brief Prototype of a context setter function
*
* @details
* A function with this prototype will be called each time a
* user callback is called, before it is called, to set the
* context in which the user callback is supposed to execute
* (this is dependent on the level of software that defines
* the info)
*
* @param[inout] cb_data: opaque pointer provided by the user
*/
typedef int (*parsec_info_set_ctx_fn)(void *cb_data);

/**
* @brief The descriptor of a single info
*/
Expand All @@ -100,7 +114,7 @@ struct parsec_info_entry_s {

/**
* @brief An array of info objects
*
*
* @details This structure holds the info objects
* in an array indexed by the iid
*/
Expand All @@ -113,6 +127,8 @@ struct parsec_info_object_array_s {
void **info_objects; /**< Info objects are stored in this array indexed by
* info_entry->iid */
void *cons_obj; /**< obj pointer for the constructor */
parsec_info_set_ctx_fn ctx_set; /**< Callback to set the context of the ioa */
void *ctx_set_obj; /**< Parameter passed to ctx_set */
};

PARSEC_OBJ_CLASS_DECLARATION(parsec_info_object_array_t);
Expand Down Expand Up @@ -150,7 +166,7 @@ parsec_info_id_t parsec_info_register(parsec_info_t *nfo, const char *name,
parsec_info_destructor_t destructor, void *des_data,
parsec_info_constructor_t constructor, void *cons_data,
void *cb_data);

/**
* @brief unregisters an info key using its ID.
*
Expand Down Expand Up @@ -183,14 +199,17 @@ parsec_info_id_t parsec_info_lookup(parsec_info_t *nfo, const char *name, void *
* @param[IN] nfo: the info collection that defines its keys
* @param[IN] cons_obj: pointer to the object holding the object array (passed as
* first argument to constructor calls)
* @param[IN] ctx_set: how to set the context for this ioa
* @param[IN] ctx_set_param: what parameter to pass to ctx_set
*
* @remark when constructing an object array with PARSE_OBJ_CONSTRUCT or PARSEC_OBJ_NEW,
* the parsec_info_t cannot be associated to the object array automatically as the constructor
* do not take parameters. It is thus needed to initialize the object_array with the info
* after it is constructed.
*/
void parsec_info_object_array_init(parsec_info_object_array_t *oa, parsec_info_t *nfo,
void *cons_obj);
void *cons_obj, parsec_info_set_ctx_fn ctx_set,
void *ctx_set_obj);

/**
* @brief Set an info in an array of objects
Expand Down Expand Up @@ -235,6 +254,19 @@ void *parsec_info_test_and_set(parsec_info_object_array_t *oa, parsec_info_id_t
*/
void *parsec_info_get(parsec_info_object_array_t *oa, parsec_info_id_t info_id);

/**
* @brief (pre)set the info object for all registered stream/device/...
*
* @details
* @param[IN] nfo: the info collection that needs to be pre-set
* @param[IN] iid: the index of the info to set
* This will call @fn parsec_info_get on each stream or device registered
* with @p nfo for the index @p iid, potentially calling the @p constructor
* callback for the objects that do not have an entry in the object array
* already.
*/
void parsec_info_set_all(parsec_info_t *nfo, parsec_info_id_t iid);


END_C_DECLS
#endif /* PARSEC_INFO_H_HAS_BEEN_INCLUDED */
2 changes: 1 addition & 1 deletion parsec/mca/device/cuda/device_cuda_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ parsec_cuda_module_init( int dev_id, parsec_device_module_t** module )
{goto release_device;} );
exec_stream->workspace = NULL;
PARSEC_OBJ_CONSTRUCT(&exec_stream->infos, parsec_info_object_array_t);
parsec_info_object_array_init(&exec_stream->infos, &parsec_per_stream_infos, exec_stream);
parsec_info_object_array_init(&exec_stream->infos, &parsec_per_stream_infos, exec_stream, (parsec_info_set_ctx_fn)parsec_cuda_set_device, gpu_device);
exec_stream->max_events = PARSEC_MAX_EVENTS_PER_STREAM;
exec_stream->executed = 0;
exec_stream->start = 0;
Expand Down
14 changes: 13 additions & 1 deletion parsec/mca/device/device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "parsec/execution_stream.h"
#include "parsec/utils/argv.h"
#include "parsec/parsec_internal.h"
#include "parsec/mca/device/device_gpu.h"

#include <stdlib.h>
#if defined(PARSEC_HAVE_ERRNO_H)
Expand Down Expand Up @@ -1022,6 +1023,17 @@ int parsec_mca_device_attach(parsec_context_t* context)
return PARSEC_SUCCESS;
}

static int parsec_device_set_ctx(void *_device)
{
parsec_device_module_t *device = (parsec_device_module_t*)_device;
if(PARSEC_DEV_IS_GPU(device->type)) {
parsec_device_gpu_module_t *gpu_device = (parsec_device_gpu_module_t *)device;
gpu_device->set_device(gpu_device);
}
/* Nothing to do for non-GPU devices */
return PARSEC_SUCCESS;
}

int parsec_mca_device_add(parsec_context_t* context, parsec_device_module_t* device)
{
if( parsec_mca_device_are_freezed ) {
Expand All @@ -1045,7 +1057,7 @@ int parsec_mca_device_add(parsec_context_t* context, parsec_device_module_t* dev
device->context = context;
parsec_atomic_unlock(&parsec_devices_mutex); /* CRITICAL SECTION: END */
PARSEC_OBJ_CONSTRUCT(&device->infos, parsec_info_object_array_t);
parsec_info_object_array_init(&device->infos, &parsec_per_device_infos, device);
parsec_info_object_array_init(&device->infos, &parsec_per_device_infos, device, parsec_device_set_ctx, device);
return device->device_index;
}

Expand Down
2 changes: 1 addition & 1 deletion parsec/mca/device/level_zero/device_level_zero_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ int parsec_level_zero_module_init( int dev_id, parsec_device_level_zero_driver_t
PARSEC_LEVEL_ZERO_CHECK_ERROR( "zeCommandQueueCreate ", ze_rc, {goto release_device;} );
exec_stream->workspace = NULL;
PARSEC_OBJ_CONSTRUCT(&exec_stream->infos, parsec_info_object_array_t);
parsec_info_object_array_init(&exec_stream->infos, &parsec_per_stream_infos, exec_stream);
parsec_info_object_array_init(&exec_stream->infos, &parsec_per_stream_infos, exec_stream, (parsec_info_set_ctx_fn)parsec_level_zero_set_device, gpu_device);
exec_stream->max_events = PARSEC_MAX_EVENTS_PER_STREAM;
exec_stream->executed = 0;
exec_stream->start = 0;
Expand Down
2 changes: 2 additions & 0 deletions tests/runtime/cuda/nvlink_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ parsec_taskpool_t* testing_nvlink_New( parsec_context_t *ctx, int depth, int mb
create_cublas_handle, NULL,
NULL);
assert(CuHI != -1);
/* Pre-set the cublas handle for all streams */
parsec_info_set_all(&parsec_per_stream_infos, CuHI);
#else
int CuHI = -1;
#endif
Expand Down
Loading