Skip to content

Commit

Permalink
bugfix: handle the case where the adt is persistent
Browse files Browse the repository at this point in the history
  • Loading branch information
abouteiller committed Apr 23, 2024
1 parent 5a5aa5f commit 193050d
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 46 deletions.
21 changes: 17 additions & 4 deletions parsec/data.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,8 @@ parsec_data_destroy( parsec_data_t *data )

/* adt constructor is split in two stages: when the adt is constructed
* with OBJ_CONSTRUCT (or allocated with OBJ_NEW), we initialize the
* adt object. The user code must then call the parameterized constructor
* adt object with static zero values.
* The user code must then call the parameterized constructor
* (below) that attaches the datatype and creates the associated arena.
*
* When the adt object is destructed (or released), the implicit destructor
Expand All @@ -583,11 +584,23 @@ int parsec_arena_datatype_construct(parsec_arena_datatype_t *adt,
adt->arena = PARSEC_OBJ_NEW(parsec_arena_t);
parsec_arena_construct(adt->arena, elem_size,
alignment);
adt->opaque_dtt = opaque_dtt;
return PARSEC_SUCCESS;
}

/* This constructor must only initialize fields with static values.
* When the user is using a persistent adt, the PTG generator will
* construct_zero the static arenas, and the user will overwrite them
* without calling destruct, which is valid only if this function does
* not allocate memory.
*/
static void parsec_arena_datatype_construct_zero(parsec_object_t *obj) {
parsec_arena_datatype_t *adt = (parsec_arena_datatype_t *)obj;
adt->arena = NULL;
adt->ht_item.next_item = NULL; /* keep Coverity happy */
adt->ht_item.hash64 = 0; /* keep Coverity happy */
adt->ht_item.key = 0; /* keep Coverity happy */
adt->opaque_dtt = opaque_dtt;
return PARSEC_SUCCESS;
adt->opaque_dtt = NULL;
}

static void parsec_arena_datatype_destruct(parsec_object_t *obj) {
Expand All @@ -596,5 +609,5 @@ static void parsec_arena_datatype_destruct(parsec_object_t *obj) {
}

PARSEC_OBJ_CLASS_INSTANCE(parsec_arena_datatype_t, parsec_object_t,
NULL,
parsec_arena_datatype_construct_zero,
parsec_arena_datatype_destruct);
7 changes: 4 additions & 3 deletions parsec/data_dist/matrix/matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,10 @@ int parsec_matrix_arena_datatype_destruct_free_type(parsec_arena_datatype_t *adt
parsec_matrix_arena_datatype_construct_alloc_type( (_adt_), (_oldtype_), PARSEC_MATRIX_FULL, 0, (_m_), (_n_), (_ld_), PARSEC_ARENA_ALIGNMENT_SSE, -1 ), \
(_adt_))

#define parsec_matrix_adt_free(_adt_) do {\
parsec_matrix_arena_datatype_destruct_free_type(_adt_); \
free(_adt_); \
#define parsec_matrix_adt_free(_padt_) do {\
parsec_matrix_arena_datatype_destruct_free_type(*(_padt_)); \
free(*(_padt_)); \
(*_padt_) = NULL; \
} while(0)


Expand Down
5 changes: 4 additions & 1 deletion parsec/interfaces/ptg/ptg-compiler/jdf2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -4758,7 +4758,10 @@ static void jdf_generate_constructor( const jdf_t* jdf )
* stages, one in the generated code runs the generic constructor, then
* the user code calls the parameterized constructor (that attaches the
* datatype). Thus, for symmetry the user code is also responsible for calling
* the destructor. */
* the destructor.
* The statically constructed arena_datatype_t can be overwritten by
* the user without leaking memory.
*/
coutput(" for(i = 0; i < (uint32_t)__parsec_tp->super.arenas_datatypes_size; i++) {\n"
" PARSEC_OBJ_CONSTRUCT(&__parsec_tp->super.arenas_datatypes[i], parsec_arena_datatype_t);\n"
" }\n");
Expand Down
13 changes: 5 additions & 8 deletions tests/apps/haar_tree/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ int main(int argc, char *argv[])
parsec_matrix_block_cyclic_t fakeDesc;
parsec_project_taskpool_t *project;
parsec_walk_taskpool_t *walker;
parsec_arena_datatype_t adt;
parsec_arena_datatype_t *adt = PARSEC_OBJ_NEW(parsec_arena_datatype_t);
int do_checks = 0, be_verbose = 0;
int pargc = 0, i;
char **pargv;
Expand Down Expand Up @@ -216,24 +216,22 @@ int main(int argc, char *argv[])
0, 0, world, world,
1, world, 1, 1, 0, 0);

parsec_matrix_adt_construct_rect( &adt,
parsec_matrix_adt_construct_rect( adt,
parsec_datatype_float_t, 2, 1, 2);

#if defined(HAVE_MPI)
MPI_Barrier(MPI_COMM_WORLD);
#endif

project = parsec_project_new(treeA, world, (parsec_data_collection_t*)&fakeDesc, 1e-3, be_verbose, 1.0);
project->arenas_datatypes[PARSEC_project_DEFAULT_ADT_IDX] = adt;
PARSEC_OBJ_RETAIN(adt.arena);
project->arenas_datatypes[PARSEC_project_DEFAULT_ADT_IDX] = *adt;
rc = parsec_context_add_taskpool(parsec, &project->super);
PARSEC_CHECK_ERROR(rc, "parsec_context_add_taskpool");
rc = parsec_context_start(parsec);
PARSEC_CHECK_ERROR(rc, "parsec_context_start");
rc = parsec_context_wait(parsec);
PARSEC_CHECK_ERROR(rc, "parsec_context_wait");

project->arenas_datatypes[PARSEC_project_DEFAULT_ADT_IDX].arena = NULL;
parsec_taskpool_free(&project->super);
ret = 0;

Expand All @@ -247,7 +245,7 @@ int main(int argc, char *argv[])
rs, print_node_fn, print_link_fn,
be_verbose);
}
walker->arenas_datatypes[PARSEC_walk_DEFAULT_ADT_IDX] = adt;
walker->arenas_datatypes[PARSEC_walk_DEFAULT_ADT_IDX] = *adt;
rc = parsec_context_add_taskpool(parsec, &walker->super);
PARSEC_CHECK_ERROR(rc, "parsec_context_add_taskpool");
rc = parsec_context_start(parsec);
Expand Down Expand Up @@ -291,9 +289,8 @@ int main(int argc, char *argv[])
}
}
#endif /* defined(HAVE_MPI) */
parsec_matrix_arena_datatype_destruct_free_type( & adt );
parsec_matrix_adt_free( &adt );

walker->arenas_datatypes[PARSEC_walk_DEFAULT_ADT_IDX].arena = NULL;
parsec_taskpool_free(&walker->super);

tree_dist_free(treeA);
Expand Down
3 changes: 3 additions & 0 deletions tests/collections/reshape/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ int reshape_print(parsec_execution_stream_t *es,
parsec_arena_datatype_t adt_default; \
parsec_arena_datatype_t adt_lower; \
parsec_arena_datatype_t adt_upper; \
PARSEC_OBJ_CONSTRUCT(&adt_default, parsec_arena_datatype_t); \
PARSEC_OBJ_CONSTRUCT(&adt_lower, parsec_arena_datatype_t); \
PARSEC_OBJ_CONSTRUCT(&adt_upper, parsec_arena_datatype_t); \
parsec_matrix_adt_construct_rect( &adt_default, \
parsec_datatype_int_t, MB, NB, MB ); \
parsec_matrix_adt_construct_lower( &adt_lower, \
Expand Down
1 change: 0 additions & 1 deletion tests/collections/reshape/testing_avoidable_reshape.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ int main(int argc, char *argv[])
parsec_datatype_t tmp = adt_default.opaque_dtt;
ctp->arenas_datatypes[PARSEC_avoidable_reshape_DEFAULT_ADT_IDX].opaque_dtt = dcA.super.super.default_dtt;
#endif
PARSEC_OBJ_RETAIN(adt_default.arena);

DO_RUN(ctp);
DO_CHECK(avoidable_reshape, dcA, dcA_check);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ int main(int argc, char *argv[])
ctp->arenas_datatypes[PARSEC_remote_multiple_outs_same_pred_flow_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_remote_multiple_outs_same_pred_flow_LOWER_TILE_ADT_IDX] = adt_lower;
ctp->arenas_datatypes[PARSEC_remote_multiple_outs_same_pred_flow_UPPER_TILE_ADT_IDX] = adt_upper;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);
PARSEC_OBJ_RETAIN(adt_upper.arena);

DO_RUN(ctp);
DO_CHECK(remote_multiple_outs_same_pred_flow, dcA, dcA_check);
Expand Down
12 changes: 0 additions & 12 deletions tests/collections/reshape/testing_reshape.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_local_no_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_local_no_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(local_no_reshape, dcA, dcA_check);
Expand Down Expand Up @@ -124,8 +122,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_local_read_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_local_read_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(local_read_reshape, dcA, dcA_check);
Expand Down Expand Up @@ -157,8 +153,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_local_output_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_local_output_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(local_output_reshape, dcA, dcA_check);
Expand Down Expand Up @@ -190,8 +184,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_local_input_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_local_input_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(local_input_reshape, dcA, dcA_check);
Expand Down Expand Up @@ -222,8 +214,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_remote_read_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_remote_read_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(remote_read_reshape, dcA, dcA_check);
Expand Down Expand Up @@ -252,8 +242,6 @@ int main(int argc, char *argv[])

ctp->arenas_datatypes[PARSEC_remote_no_re_reshape_DEFAULT_ADT_IDX] = adt_default;
ctp->arenas_datatypes[PARSEC_remote_no_re_reshape_LOWER_TILE_ADT_IDX] = adt_lower;
PARSEC_OBJ_RETAIN(adt_default.arena);
PARSEC_OBJ_RETAIN(adt_lower.arena);

DO_RUN(ctp);
DO_CHECK(remote_no_re_reshape, dcA, dcA_check);
Expand Down
11 changes: 5 additions & 6 deletions tests/dsl/ptg/complex_deps.jdf
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ int main( int argc, char** argv )
{
parsec_complex_deps_taskpool_t* tp;
parsec_matrix_block_cyclic_t descA;
parsec_arena_datatype_t adt;
parsec_arena_datatype_t *adt = PARSEC_OBJ_NEW(parsec_arena_datatype_t);
parsec_datatype_t otype;
parsec_context_t *parsec;
int ni = NN, nk = NN, verbose = 0, i = 1, rc;
Expand Down Expand Up @@ -167,15 +167,14 @@ int main( int argc, char** argv )
descA.super.bsiz *
parsec_datadist_getsizeoftype(TYPE) );
parsec_translate_matrix_type(TYPE, &otype);
parsec_matrix_adt_construct_rect(&adt, otype,
parsec_matrix_adt_construct_rect(adt, otype,
descA.super.mb, descA.super.nb, descA.super.mb);


/* Heat up the engine: small tasks */
tp = parsec_complex_deps_new( &descA, ni, nk );
assert( NULL != tp );
tp->arenas_datatypes[PARSEC_complex_deps_DEFAULT_ADT_IDX] = adt;
PARSEC_OBJ_RETAIN(adt.arena);
tp->arenas_datatypes[PARSEC_complex_deps_DEFAULT_ADT_IDX] = *adt;

rc = parsec_context_add_taskpool( parsec, (parsec_taskpool_t*)tp );
PARSEC_CHECK_ERROR(rc, "parsec_context_add_taskpool");
Expand All @@ -191,7 +190,7 @@ int main( int argc, char** argv )
tp = parsec_complex_deps_new( &descA, ni, nk );
assert( NULL != tp );
TIMER_START(time_elapsed);
tp->arenas_datatypes[PARSEC_complex_deps_DEFAULT_ADT_IDX] = adt;
tp->arenas_datatypes[PARSEC_complex_deps_DEFAULT_ADT_IDX] = *adt;
rc = parsec_context_add_taskpool( parsec, (parsec_taskpool_t*)tp );
PARSEC_CHECK_ERROR(rc, "parsec_context_add_taskpool");
TIMER_STOP(time_elapsed);
Expand All @@ -213,7 +212,7 @@ int main( int argc, char** argv )
TIMER_STOP(time_elapsed);
printf("DAG execution in %ld micro-sec\n", time_elapsed);

parsec_matrix_arena_datatype_destruct_free_type( & adt );
parsec_matrix_adt_free( & adt );

free(descA.mat);

Expand Down
14 changes: 6 additions & 8 deletions tests/dsl/ptg/startup.jdf
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ int main( int argc, char** argv )
{
parsec_startup_taskpool_t* tp;
parsec_matrix_block_cyclic_t descA;
parsec_arena_datatype_t adt;
parsec_arena_datatype_t *adt = PARSEC_OBJ_NEW(parsec_arena_datatype_t);
parsec_datatype_t dt;
parsec_context_t *parsec;
int ni = NN, nj = NN, nk = NN, verbose = 0, i = 1, rc;
Expand Down Expand Up @@ -145,7 +145,7 @@ int main( int argc, char** argv )
parsec_datadist_getsizeoftype(TYPE) );

parsec_translate_matrix_type(TYPE, &dt);
parsec_matrix_adt_construct_rect(&adt, dt,
parsec_matrix_adt_construct_rect(adt, dt,
descA.super.mb, descA.super.nb, descA.super.mb);

srandom((int)getpid()); /* Start the random generator */
Expand All @@ -157,10 +157,9 @@ int main( int argc, char** argv )
/* Heat up the engine: small tasks no priority */
tp = parsec_startup_new( &descA, ni, nj, nk );
assert( NULL != tp );
tp->arenas_datatypes[PARSEC_startup_DEFAULT_ADT_IDX] = adt;
PARSEC_OBJ_RETAIN(adt.arena);

tp->arenas_datatypes[PARSEC_startup_DEFAULT_ADT_IDX] = *adt;
tp->_g_pri = 0;

rc = parsec_context_add_taskpool( parsec, (parsec_taskpool_t*)tp );
PARSEC_CHECK_ERROR(rc, "parsec_context_add_taskpool");
rc = parsec_context_wait(parsec);
Expand All @@ -177,8 +176,7 @@ int main( int argc, char** argv )
assert( NULL != tp );

TIMER_START(time_elapsed);
tp->arenas_datatypes[PARSEC_startup_DEFAULT_ADT_IDX] = adt;
PARSEC_OBJ_RETAIN(adt.arena);
tp->arenas_datatypes[PARSEC_startup_DEFAULT_ADT_IDX] = *adt;
tp->_g_pri = priorities[i].prio;

rc = parsec_context_add_taskpool( parsec, (parsec_taskpool_t*)tp );
Expand All @@ -203,7 +201,7 @@ int main( int argc, char** argv )
}

free(descA.mat);
parsec_matrix_arena_datatype_destruct_free_type( & adt );
parsec_matrix_adt_free(&adt);

parsec_fini( &parsec);

Expand Down

0 comments on commit 193050d

Please sign in to comment.