Interesting, this issue exists in 2 out of 3 functions defined in the
ompi_datatype_create_indexed.c file. Based on your patch I create one that
fixes all the issues with the indexed type creation. Attached is the patch.
I'll push it in the trunk and create CMRs.
Thanks,
george.
Index: ompi/datatype/ompi_datatype_create_indexed.c
===================================================================
--- ompi/datatype/ompi_datatype_create_indexed.c (revision 24616)
+++ ompi/datatype/ompi_datatype_create_indexed.c (working copy)
@@ -3,7 +3,7 @@
* Copyright (c) 2004-2006 The Trustees of Indiana University and Indiana
* University Research and Technology
* Corporation. All rights reserved.
- * Copyright (c) 2004-2009 The University of Tennessee and The University
+ * Copyright (c) 2004-2010 The University of Tennessee and The University
* of Tennessee Research Foundation. All rights
* reserved.
* Copyright (c) 2004-2006 High Performance Computing Center Stuttgart,
@@ -46,26 +46,21 @@
dLength = pBlockLength[0];
endat = disp + dLength;
ompi_datatype_type_extent( oldType, &extent );
- if( 1 >= count ) {
- pdt = ompi_datatype_create( oldType->super.desc.used + 2 );
- /* multiply by count to make it zero if count is zero */
- ompi_datatype_add( pdt, oldType, count * dLength, disp * extent,
extent );
- } else {
- pdt = ompi_datatype_create( count * (2 + oldType->super.desc.used) );
- for( i = 1; i < count; i++ ) {
- if( endat == pDisp[i] ) {
- /* contiguous with the previsious */
- dLength += pBlockLength[i];
- endat += pBlockLength[i];
- } else {
- ompi_datatype_add( pdt, oldType, dLength, disp * extent,
extent );
- disp = pDisp[i];
- dLength = pBlockLength[i];
- endat = disp + pBlockLength[i];
- }
+
+ pdt = ompi_datatype_create( count * (2 + oldType->super.desc.used) );
+ for( i = 1; i < count; i++ ) {
+ if( endat == pDisp[i] ) {
+ /* contiguous with the previsious */
+ dLength += pBlockLength[i];
+ endat += pBlockLength[i];
+ } else {
+ ompi_datatype_add( pdt, oldType, dLength, disp * extent, extent );
+ disp = pDisp[i];
+ dLength = pBlockLength[i];
+ endat = disp + pBlockLength[i];
}
- ompi_datatype_add( pdt, oldType, dLength, disp * extent, extent );
}
+ ompi_datatype_add( pdt, oldType, dLength, disp * extent, extent );
*newType = pdt;
return OMPI_SUCCESS;
@@ -91,25 +86,20 @@
dLength = pBlockLength[0];
endat = disp + dLength * extent;
- if( 1 >= count ) {
- pdt = ompi_datatype_create( oldType->super.desc.used + 2 );
- /* multiply by count to make it zero if count is zero */
- ompi_datatype_add( pdt, oldType, count * dLength, disp, extent );
- } else {
- for( i = 1; i < count; i++ ) {
- if( endat == pDisp[i] ) {
- /* contiguous with the previsious */
- dLength += pBlockLength[i];
- endat += pBlockLength[i] * extent;
- } else {
- ompi_datatype_add( pdt, oldType, dLength, disp, extent );
- disp = pDisp[i];
- dLength = pBlockLength[i];
- endat = disp + pBlockLength[i] * extent;
- }
+ for( i = 1; i < count; i++ ) {
+ if( endat == pDisp[i] ) {
+ /* contiguous with the previsious */
+ dLength += pBlockLength[i];
+ endat += pBlockLength[i] * extent;
+ } else {
+ ompi_datatype_add( pdt, oldType, dLength, disp, extent );
+ disp = pDisp[i];
+ dLength = pBlockLength[i];
+ endat = disp + pBlockLength[i] * extent;
}
- ompi_datatype_add( pdt, oldType, dLength, disp, extent );
}
+ ompi_datatype_add( pdt, oldType, dLength, disp, extent );
+
*newType = pdt;
return OMPI_SUCCESS;
}
On Apr 14, 2011, at 10:48 , Pascal Deveze wrote:
> Calling MPI_Type_create_hindexed(int count, int array_of_blocklengths[],
> MPI_Aint array_of_displacements[], MPI_Datatype oldtype,
> MPI_Datatype *newtype)
> with a count parameter of 1 causes a loss of memory detected by valgrind :
>
> ==2053== 576 (448 direct, 128 indirect) bytes in 1 blocks are definitely lost
> in loss record 157 of 182
> ==2053== at 0x4C2415D: malloc (vg_replace_malloc.c:195)
> ==2053== by 0x4E7CEC7: opal_obj_new (opal_object.h:469)
> ==2053== by 0x4E7D134: ompi_datatype_create (ompi_datatype_create.c:71)
> ==2053== by 0x4E7D58E: ompi_datatype_create_hindexed
> (ompi_datatype_create_indexed.c:89)
> ==2053== by 0x4EA74D0: PMPI_Type_create_hindexed
> (ptype_create_hindexed.c:75)
> ==2053== by 0x401A5C: main (in /home_nfs/xxx/type_create_hindexed)
>
> This can be reproduced with the following trivial code:
> =====================================
> #include "mpi.h"
>
> MPI_Datatype newtype;
> int lg[3];
> MPI_Aint disp[3];
>
> int main(int argc, char **argv) {
> MPI_Init(&argc,&argv);
>
> disp[0] = (MPI_Aint)disp;
> disp[1] = (MPI_Aint)disp+1;
> lg[0] = 5;
> lg[1] = 5;
>
> MPI_Type_create_hindexed(1, lg, disp, MPI_BYTE, &newtype);
> MPI_Type_free(&newtype);
>
> MPI_Finalize();
> }
> ======================================
> If MPI_Type_create_hindexed() is called with a count parameter greater 1,
> valgrind does not detect any lost record.
>
> Patch proposed:
>
> hg diff ompi/datatype/ompi_datatype_create_indexed.c
> diff -r a2d94a70f474 ompi/datatype/ompi_datatype_create_indexed.c
> --- a/ompi/datatype/ompi_datatype_create_indexed.c Wed Mar 30 18:47:31
> 2011 +0200
> +++ b/ompi/datatype/ompi_datatype_create_indexed.c Thu Apr 14 16:16:08
> 2011 +0200
> @@ -91,11 +91,6 @@
> dLength = pBlockLength[0];
> endat = disp + dLength * extent;
> - if( 1 >= count ) {
> - pdt = ompi_datatype_create( oldType->super.desc.used + 2 );
> - /* multiply by count to make it zero if count is zero */
> - ompi_datatype_add( pdt, oldType, count * dLength, disp, extent );
> - } else {
> for( i = 1; i < count; i++ ) {
> if( endat == pDisp[i] ) {
> /* contiguous with the previsious */
> @@ -109,7 +104,6 @@
> }
> }
> ompi_datatype_add( pdt, oldType, dLength, disp, extent );
> - }
> *newType = pdt;
> return OMPI_SUCCESS;
> }
>
> Explanation:
> The case (0 == count) was resolved before by returning.
> The problem is that, in the case ( 1 >= count ), ompi_datatype_create() is
> called again (it has been just called before).
> In fact the case (1 == count) is not different of the case (1 < count), so
> it is possible to just avoid the if-else statement.
>
> We need a patch for OpenMPI 1.5 branch.
>
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
"I disapprove of what you say, but I will defend to the death your right to say
it"
-- Evelyn Beatrice Hall
