Skip to content

Don't refcount the predefined datatypes. #2393

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

Merged
merged 1 commit into from
Feb 21, 2017
Merged

Don't refcount the predefined datatypes. #2393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 21 additions & 0 deletions ompi/datatype/ompi_datatype.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

#include "ompi/constants.h"
#include "opal/datatype/opal_convertor.h"
#include "opal/util/output.h"
#include "mpi.h"

BEGIN_C_DECLS
Expand Down Expand Up @@ -374,5 +375,25 @@ OMPI_DECLSPEC int ompi_datatype_unpack_external( const char datarep[], const voi
OMPI_DECLSPEC int ompi_datatype_pack_external_size( const char datarep[], int incount,
ompi_datatype_t *datatype, MPI_Aint *size);

#define OMPI_DATATYPE_RETAIN(ddt) \
{ \
if( !ompi_datatype_is_predefined((ddt)) ) { \
OBJ_RETAIN((ddt)); \
OPAL_OUTPUT_VERBOSE((0, 100, "Datatype %p [%s] refcount %d in file %s:%d\n", \
(void*)(ddt), (ddt)->name, (ddt)->super.super.obj_reference_count, \
__FILE__, __LINE__)); \
} \
}

#define OMPI_DATATYPE_RELEASE(ddt) \
{ \
if( !ompi_datatype_is_predefined((ddt)) ) { \
OPAL_OUTPUT_VERBOSE((0, 100, "Datatype %p [%s] refcount %d in file %s:%d\n", \
Copy link
Contributor

Choose a reason for hiding this comment

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

@bosilca fwiw, you might or might not want to print
(ddt)->super.super.obj_reference_count - 1 instead

i do not really have a strong opinion on that though

Copy link
Member Author

Choose a reason for hiding this comment

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

This message reports non-thread safe data, with the exception of developers nobody should ever use it. Let's keep it as is.

(void*)(ddt), (ddt)->name, (ddt)->super.super.obj_reference_count, \
__func__, __LINE__)); \
OBJ_RELEASE((ddt)); \
} \
}

END_C_DECLS
#endif /* OMPI_DATATYPE_H_HAS_BEEN_INCLUDED */
2 changes: 1 addition & 1 deletion ompi/mca/osc/base/osc_base_obj_convert.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ompi_osc_base_datatype_create(ompi_proc_t *remote_proc, void **payload)
struct ompi_datatype_t *datatype =
ompi_datatype_create_from_packed_description(payload, remote_proc);
if (NULL == datatype) return NULL;
if (ompi_datatype_is_predefined(datatype)) OBJ_RETAIN(datatype);
OMPI_DATATYPE_RETAIN(datatype);
return datatype;
}

Expand Down
16 changes: 8 additions & 8 deletions ompi/mca/osc/pt2pt/osc_pt2pt_comm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static int ompi_osc_pt2pt_dt_send_complete (ompi_request_t *request)
ompi_datatype_t *datatype = (ompi_datatype_t *) request->req_complete_cb_data;
ompi_osc_pt2pt_module_t *module = NULL;

OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

OPAL_THREAD_LOCK(&mca_osc_pt2pt_component.lock);
(void) opal_hash_table_get_value_uint32(&mca_osc_pt2pt_component.modules,
Expand Down Expand Up @@ -360,7 +360,7 @@ static inline int ompi_osc_pt2pt_put_w_req (const void *origin_addr, int origin_
/* the datatype does not fit in an eager message. send it seperately */
header->base.flags |= OMPI_OSC_PT2PT_HDR_FLAG_LARGE_DATATYPE;

OBJ_RETAIN(target_dt);
OMPI_DATATYPE_RETAIN(target_dt);

ret = ompi_osc_pt2pt_isend_w_cb ((void *) packed_ddt, ddt_len, MPI_BYTE,
target, tag_to_target(tag), module->comm,
Expand Down Expand Up @@ -516,7 +516,7 @@ ompi_osc_pt2pt_accumulate_w_req (const void *origin_addr, int origin_count,
/* the datatype does not fit in an eager message. send it seperately */
header->base.flags |= OMPI_OSC_PT2PT_HDR_FLAG_LARGE_DATATYPE;

OBJ_RETAIN(target_dt);
OMPI_DATATYPE_RETAIN(target_dt);

ret = ompi_osc_pt2pt_isend_w_cb ((void *) packed_ddt, ddt_len, MPI_BYTE,
target, tag_to_target(tag), module->comm,
Expand Down Expand Up @@ -620,7 +620,7 @@ int ompi_osc_pt2pt_compare_and_swap (const void *origin_addr, const void *compar
request->type = OMPI_OSC_PT2PT_HDR_TYPE_CSWAP;
request->origin_addr = origin_addr;
request->internal = true;
OBJ_RETAIN(dt);
OMPI_DATATYPE_RETAIN(dt);
request->origin_dt = dt;

/* Compute datatype and payload lengths. Note that the datatype description
Expand Down Expand Up @@ -775,7 +775,7 @@ static inline int ompi_osc_pt2pt_rget_internal (void *origin_addr, int origin_co
pt2pt_request->type = OMPI_OSC_PT2PT_HDR_TYPE_GET;
pt2pt_request->origin_addr = origin_addr;
pt2pt_request->origin_count = origin_count;
OBJ_RETAIN(origin_dt);
OMPI_DATATYPE_RETAIN(origin_dt);
pt2pt_request->origin_dt = origin_dt;

/* Compute datatype length. Note that the datatype description
Expand Down Expand Up @@ -825,7 +825,7 @@ static inline int ompi_osc_pt2pt_rget_internal (void *origin_addr, int origin_co
/* the datatype does not fit in an eager message. send it seperately */
header->base.flags |= OMPI_OSC_PT2PT_HDR_FLAG_LARGE_DATATYPE;

OBJ_RETAIN(target_dt);
OMPI_DATATYPE_RETAIN(target_dt);

ret = ompi_osc_pt2pt_isend_w_cb ((void *) packed_ddt, ddt_len, MPI_BYTE,
target, tag_to_target(tag), module->comm,
Expand Down Expand Up @@ -983,7 +983,7 @@ int ompi_osc_pt2pt_rget_accumulate_internal (const void *origin_addr, int origin
pt2pt_request->type = OMPI_OSC_PT2PT_HDR_TYPE_GET_ACC;
pt2pt_request->origin_addr = origin_addr;
pt2pt_request->origin_count = origin_count;
OBJ_RETAIN(origin_datatype);
OMPI_DATATYPE_RETAIN(origin_datatype);
pt2pt_request->origin_dt = origin_datatype;

/* Compute datatype and payload lengths. Note that the datatype description
Expand Down Expand Up @@ -1044,7 +1044,7 @@ int ompi_osc_pt2pt_rget_accumulate_internal (const void *origin_addr, int origin
/* the datatype does not fit in an eager message. send it seperately */
header->base.flags |= OMPI_OSC_PT2PT_HDR_FLAG_LARGE_DATATYPE;

OBJ_RETAIN(target_datatype);
OMPI_DATATYPE_RETAIN(target_datatype);

ret = ompi_osc_pt2pt_isend_w_cb ((void *) packed_ddt, ddt_len, MPI_BYTE,
target_rank, tag_to_target(tag), module->comm,
Expand Down
26 changes: 13 additions & 13 deletions ompi/mca/osc/pt2pt/osc_pt2pt_data_move.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static void osc_pt2pt_accumulate_data_destructor (osc_pt2pt_accumulate_data_t *a
}

if (acc_data->datatype) {
OBJ_RELEASE(acc_data->datatype);
OMPI_DATATYPE_RELEASE(acc_data->datatype);
}
}

Expand Down Expand Up @@ -118,7 +118,7 @@ static void osc_pt2pt_pending_acc_destructor (osc_pt2pt_pending_acc_t *pending)
}

if (NULL != pending->datatype) {
OBJ_RELEASE(pending->datatype);
OMPI_DATATYPE_RELEASE(pending->datatype);
}
}

Expand Down Expand Up @@ -370,7 +370,7 @@ static inline int process_put(ompi_osc_pt2pt_module_t* module, int source,

osc_pt2pt_copy_on_recv (target, data, data_len, proc, put_header->count, datatype);

OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return put_header->len;
}
Expand Down Expand Up @@ -407,7 +407,7 @@ static inline int process_put_long(ompi_osc_pt2pt_module_t* module, int source,
return OMPI_ERROR;
}

OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return put_header->len;
}
Expand Down Expand Up @@ -532,7 +532,7 @@ static inline int process_get (ompi_osc_pt2pt_module_t* module, int target,
ret = osc_pt2pt_get_post_send (module, source, get_header->count, datatype,
target, tag_to_origin(get_header->tag));

OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return OMPI_SUCCESS == ret ? (int) get_header->len : ret;
}
Expand Down Expand Up @@ -634,7 +634,7 @@ static int osc_pt2pt_accumulate_allocate (ompi_osc_pt2pt_module_t *module, int p
acc_data->proc = proc;
acc_data->count = count;
acc_data->datatype = datatype;
OBJ_RETAIN(datatype);
OMPI_DATATYPE_RETAIN(datatype);
acc_data->op = op;
acc_data->request_count = request_count;

Expand Down Expand Up @@ -728,7 +728,7 @@ static int ompi_osc_pt2pt_acc_op_queue (ompi_osc_pt2pt_module_t *module, ompi_os

/* save the datatype */
pending_acc->datatype = datatype;
OBJ_RETAIN(datatype);
OMPI_DATATYPE_RETAIN(datatype);

/* save the header */
switch (header->base.type) {
Expand Down Expand Up @@ -1166,7 +1166,7 @@ static inline int process_acc (ompi_osc_pt2pt_module_t *module, int source,
}

/* Release datatype & op */
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return (OMPI_SUCCESS == ret) ? (int) acc_header->len : ret;
}
Expand Down Expand Up @@ -1197,7 +1197,7 @@ static inline int process_acc_long (ompi_osc_pt2pt_module_t* module, int source,
}

/* Release datatype & op */
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return (OMPI_SUCCESS == ret) ? (int) acc_header->len : ret;
}
Expand Down Expand Up @@ -1231,7 +1231,7 @@ static inline int process_get_acc(ompi_osc_pt2pt_module_t *module, int source,
uint32_t primitive_count;
buffer = malloc (data_len);
if (OPAL_UNLIKELY(NULL == buffer)) {
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);
return OMPI_ERR_OUT_OF_RESOURCE;
}

Expand All @@ -1250,7 +1250,7 @@ static inline int process_get_acc(ompi_osc_pt2pt_module_t *module, int source,
}

/* Release datatype & op */
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return (OMPI_SUCCESS == ret) ? (int) acc_header->len : ret;
}
Expand Down Expand Up @@ -1281,7 +1281,7 @@ static inline int process_get_acc_long(ompi_osc_pt2pt_module_t *module, int sour
}

/* Release datatype & op */
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return OMPI_SUCCESS == ret ? (int) acc_header->len : ret;
}
Expand Down Expand Up @@ -1313,7 +1313,7 @@ static inline int process_cswap (ompi_osc_pt2pt_module_t *module, int source,
}

/* Release datatype */
OBJ_RELEASE(datatype);
OMPI_DATATYPE_RELEASE(datatype);

return (OMPI_SUCCESS == ret) ? (int) cswap_header->len : ret;
}
Expand Down
4 changes: 2 additions & 2 deletions ompi/mca/pml/base/pml_base_recvreq.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION(mca_pml_base_recv_request_t);
{ \
/* increment reference count on communicator */ \
OBJ_RETAIN(comm); \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
\
OMPI_REQUEST_INIT(&(request)->req_base.req_ompi, persistent); \
(request)->req_base.req_ompi.req_mpi_object.comm = comm; \
Expand Down Expand Up @@ -117,7 +117,7 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION(mca_pml_base_recv_request_t);
do { \
OMPI_REQUEST_FINI(&(request)->req_base.req_ompi); \
OBJ_RELEASE( (request)->req_base.req_comm); \
OBJ_RELEASE( (request)->req_base.req_datatype ); \
OMPI_DATATYPE_RELEASE( (request)->req_base.req_datatype ); \
opal_convertor_cleanup( &((request)->req_base.req_convertor) ); \
} while (0)

Expand Down
4 changes: 2 additions & 2 deletions ompi/mca/pml/base/pml_base_sendreq.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION( mca_pml_base_send_request_t );
\
/* initialize datatype convertor for this request */ \
if( count > 0 ) { \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
/* We will create a convertor specialized for the */ \
/* remote architecture and prepared with the datatype. */ \
opal_convertor_copy_and_prepare_for_send( \
Expand Down Expand Up @@ -145,7 +145,7 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION( mca_pml_base_send_request_t );
OMPI_REQUEST_FINI(&(request)->req_base.req_ompi); \
OBJ_RELEASE((request)->req_base.req_comm); \
if( 0 != (request)->req_base.req_count ) \
OBJ_RELEASE((request)->req_base.req_datatype); \
OMPI_DATATYPE_RELEASE((request)->req_base.req_datatype); \
opal_convertor_cleanup( &((request)->req_base.req_convertor) ); \
} while (0)

Expand Down
12 changes: 6 additions & 6 deletions ompi/mca/pml/cm/pml_cm_recvreq.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ do { \
request->req_base.req_comm = comm; \
request->req_base.req_datatype = datatype; \
OBJ_RETAIN(comm); \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
\
if( MPI_ANY_SOURCE == src ) { \
ompi_proc = ompi_proc_local_proc; \
Expand Down Expand Up @@ -132,7 +132,7 @@ do { \
request->req_base.req_comm = comm; \
request->req_base.req_datatype = datatype; \
OBJ_RETAIN(comm); \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
\
opal_convertor_copy_and_prepare_for_recv( \
ompi_mpi_local_convertor, \
Expand Down Expand Up @@ -166,7 +166,7 @@ do { \
request->req_addr = addr; \
request->req_count = count; \
OBJ_RETAIN(comm); \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
\
if( MPI_ANY_SOURCE == src ) { \
ompi_proc = ompi_proc_local_proc; \
Expand Down Expand Up @@ -203,7 +203,7 @@ do { \
request->req_addr = addr; \
request->req_count = count; \
OBJ_RETAIN(comm); \
OBJ_RETAIN(datatype); \
OMPI_DATATYPE_RETAIN(datatype); \
\
opal_convertor_copy_and_prepare_for_recv( \
ompi_mpi_local_convertor, \
Expand Down Expand Up @@ -349,7 +349,7 @@ do { \
#define MCA_PML_CM_HVY_RECV_REQUEST_RETURN(recvreq) \
{ \
OBJ_RELEASE((recvreq)->req_base.req_comm); \
OBJ_RELEASE((recvreq)->req_base.req_datatype); \
OMPI_DATATYPE_RELEASE((recvreq)->req_base.req_datatype); \
OMPI_REQUEST_FINI(&(recvreq)->req_base.req_ompi); \
opal_convertor_cleanup( &((recvreq)->req_base.req_convertor) ); \
opal_free_list_return ( &mca_pml_base_recv_requests, \
Expand All @@ -362,7 +362,7 @@ do { \
#define MCA_PML_CM_THIN_RECV_REQUEST_RETURN(recvreq) \
{ \
OBJ_RELEASE((recvreq)->req_base.req_comm); \
OBJ_RELEASE((recvreq)->req_base.req_datatype); \
OMPI_DATATYPE_RELEASE((recvreq)->req_base.req_datatype); \
OMPI_REQUEST_FINI(&(recvreq)->req_base.req_ompi); \
opal_convertor_cleanup( &((recvreq)->req_base.req_convertor) ); \
opal_free_list_return ( &mca_pml_base_recv_requests, \
Expand Down
Loading