Clean and sync the pack and unpack functions.

- optimize handling of contiguous with gaps datatypes.
- fixes a performance issue for all datatypes with a count of 1.
- optimize the pack/unpack of contiguous with gaps datatype.
- optimize the case of blocklen == 1

Signed-off-by: George Bosilca <bosilca@icl.utk.edu>

Author: bosilca

opal/datatype/opal_convertor_raw.c

@@ -31,8 +31,8 @@
 #endif /* OPAL_ENABLE_DEBUG */
 
 /* Take a new iovec (base + len) and try to merge it with what we already
- * have. If we succeed return 0 and move forward, if not save it into a new
- * iovec location. If we need to go to a new position and we reach the end
+ * have. If we succeed return 0 and move forward, otherwise save it into a new
+ * iovec location. If we need to advance position and we reach the end
  * of the iovec array, return 1 to signal we did not saved the last iovec.
  */
 static inline int
@@ -46,7 +46,7 @@ opal_convertor_merge_iov( struct iovec* iov, uint32_t* iov_count,
             return 0;
         }  /* cannot merge, move to the next position */
         *idx = *idx + 1;
-        if( *idx == *iov_count ) return 1;  /* do not overwrite outside the iove array boundaries */
+        if( *idx == *iov_count ) return 1;  /* do not overwrite outside the iovec array boundaries */
     }
     iov[*idx].iov_base = base;
     iov[*idx].iov_len = len;

opal/datatype/opal_datatype_copy.h

@@ -51,11 +51,9 @@ static inline void _predefined_data( const dt_elem_desc_t* ELEM,
     const ddt_elem_desc_t* _elem = &((ELEM)->elem);
     unsigned char* _source = (SOURCE) + _elem->disp;
     unsigned char* _destination = (DESTINATION) + _elem->disp;
-    size_t total_count = _elem->count * _elem->blocklen;
-    size_t do_now, do_now_bytes;
+    size_t do_now = _elem->count, do_now_bytes;
 
-    assert( (COUNT) == total_count);
-    assert( total_count <= ((*SPACE) / opal_datatype_basicDatatypes[_elem->common.type]->size) );
+    assert( (COUNT) == (do_now * _elem->blocklen));
 
     /* We don't a prologue and epilogue here as we are __always__ working
      * with full copies of the data description.
@@ -64,21 +62,19 @@ static inline void _predefined_data( const dt_elem_desc_t* ELEM,
     /**
      * Compute how many full blocklen we need to do and do them.
      */
-    do_now = _elem->count;
-    if( 0 != do_now ) {
-        do_now_bytes = _elem->blocklen * opal_datatype_basicDatatypes[_elem->common.type]->size;
-        for(size_t _i = 0; _i < do_now; _i++ ) {
-            OPAL_DATATYPE_SAFEGUARD_POINTER( _source, do_now_bytes, (SOURCE_BASE),
-                                            (DATATYPE), (TOTAL_COUNT) );
-            DO_DEBUG( opal_output( 0, "copy %s( %p, %p, %" PRIsize_t " ) => space %" PRIsize_t "\n",
-                                   STRINGIFY(MEM_OP_NAME), (void*)_destination, (void*)_source, do_now_bytes, *(SPACE) ); );
-            MEM_OP( _destination, _source, do_now_bytes );
-            _destination += _elem->extent;
-            _source      += _elem->extent;
-            *(SPACE)     -= do_now_bytes;
-        }
-        (COUNT)      -= total_count;
+    do_now_bytes = _elem->blocklen * opal_datatype_basicDatatypes[_elem->common.type]->size;
+    assert( (do_now * do_now_bytes) <= (*SPACE) );
+
+    for(size_t _i = 0; _i < do_now; _i++ ) {
+        OPAL_DATATYPE_SAFEGUARD_POINTER( _source, do_now_bytes, (SOURCE_BASE),
+                                         (DATATYPE), (TOTAL_COUNT) );
+        DO_DEBUG( opal_output( 0, "copy %s( %p, %p, %" PRIsize_t " ) => space %" PRIsize_t "\n",
+                               STRINGIFY(MEM_OP_NAME), (void*)_destination, (void*)_source, do_now_bytes, *(SPACE) - _i * do_now_bytes ); );
+        MEM_OP( _destination, _source, do_now_bytes );
+        _destination += _elem->extent;
+        _source      += _elem->extent;
     }
+    *(SPACE) -= (do_now_bytes * do_now);
 }
 
 static inline void _contiguous_loop( const dt_elem_desc_t* ELEM,

opal/datatype/opal_datatype_module.c

@@ -252,7 +252,6 @@ int32_t opal_datatype_init( void )
                                                    OPAL_DATATYPE_FLAG_CONTIGUOUS |
                                                    OPAL_DATATYPE_FLAG_NO_GAPS;
         datatype->desc.desc[0].elem.common.type  = i;
-        /* datatype->desc.desc[0].elem.blocklen XXX not set at the moment, it will be needed later */
         datatype->desc.desc[0].elem.count        = 1;
         datatype->desc.desc[0].elem.blocklen     = 1;
         datatype->desc.desc[0].elem.disp         = 0;

opal/datatype/opal_datatype_pack.c

@@ -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-2016 The University of Tennessee and The University
+ * Copyright (c) 2004-2019 The University of Tennessee and The University
  *                         of Tennessee Research Foundation.  All rights
  *                         reserved.
  * Copyright (c) 2004-2006 High Performance Computing Center Stuttgart,
@@ -53,8 +53,6 @@
 #endif  /* defined(CHECKSUM) */
 
 
-#define IOVEC_MEM_LIMIT 8192
-
 /* the contig versions does not use the stack. They can easily retrieve
  * the status with just the informations from pConvertor->bConverted.
  */
@@ -68,9 +66,8 @@ opal_pack_homogeneous_contig_function( opal_convertor_t* pConv,
     unsigned char *source_base = NULL;
     uint32_t iov_count;
     size_t length = pConv->local_size - pConv->bConverted, initial_amount = pConv->bConverted;
-    ptrdiff_t initial_displ = pConv->use_desc->desc[pConv->use_desc->used].end_loop.first_elem_disp;
 
-    source_base = (pConv->pBaseBuf + initial_displ + pStack[0].disp + pStack[1].disp);
+    source_base = (pConv->pBaseBuf + pConv->pDesc->true_lb + pStack[0].disp + pStack[1].disp);
 
     /* There are some optimizations that can be done if the upper level
      * does not provide a buffer.
@@ -111,155 +108,116 @@ opal_pack_homogeneous_contig_with_gaps_function( opal_convertor_t* pConv,
                                                  uint32_t* out_size,
                                                  size_t* max_data )
 {
+    size_t remaining, length, initial_bytes_converted = pConv->bConverted;
     const opal_datatype_t* pData = pConv->pDesc;
     dt_stack_t* stack = pConv->pStack;
+    ptrdiff_t extent = pData->ub - pData->lb;
     unsigned char *user_memory, *packed_buffer;
-    uint32_t iov_count, index;
+    uint32_t idx;
     size_t i;
-    size_t bConverted, remaining, length, initial_bytes_converted = pConv->bConverted;
-    ptrdiff_t extent= pData->ub - pData->lb;
-    ptrdiff_t initial_displ = pConv->use_desc->desc[pConv->use_desc->used].end_loop.first_elem_disp;
 
+    /* The memory layout is contiguous with gaps in the begining and at the end. The datatype true_lb
+     * is the initial displacement, the size the length of the contiguous area and the extent represent
+     * how much we should jump between elements.
+     */
     assert( (pData->flags & OPAL_DATATYPE_FLAG_CONTIGUOUS) && ((ptrdiff_t)pData->size != extent) );
     DO_DEBUG( opal_output( 0, "pack_homogeneous_contig( pBaseBuf %p, iov_count %d )\n",
                            (void*)pConv->pBaseBuf, *out_size ); );
     if( stack[1].type != opal_datatype_uint1.id ) {
         stack[1].count *= opal_datatype_basicDatatypes[stack[1].type]->size;
-        stack[1].type = opal_datatype_uint1.id;
+        stack[1].type   = opal_datatype_uint1.id;
+    }
+    /* We can provide directly the pointers in the user buffers (like the convertor_raw) */
+    if( NULL == iov[0].iov_base ) {
+        user_memory = pConv->pBaseBuf + pData->true_lb;
+
+        for( idx = 0; (idx < (*out_size)) && stack[0].count; idx++ ) {
+            iov[idx].iov_base = user_memory + stack[0].disp + stack[1].disp;
+            iov[idx].iov_len  = stack[1].count;
+            COMPUTE_CSUM( iov[idx].iov_base, iov[idx].iov_len, pConv );
+
+            pConv->bConverted += stack[1].count;
+
+            stack[0].disp += extent;
+            stack[0].count--;
+            stack[1].disp  = 0;
+            stack[1].count = pData->size;  /* we might need this to update the partial
+                                            * length for the first iteration */
+        }
+        goto update_status_and_return;
     }
 
-    /* There are some optimizations that can be done if the upper level
-     * does not provide a buffer.
-     */
-    for( iov_count = 0; iov_count < (*out_size); iov_count++ ) {
+    for( idx = 0; idx < (*out_size); idx++ ) {
         /* Limit the amount of packed data to the data left over on this convertor */
         remaining = pConv->local_size - pConv->bConverted;
         if( 0 == remaining ) break;  /* we're done this time */
-        if( remaining > iov[iov_count].iov_len )
-            remaining = iov[iov_count].iov_len;
-        packed_buffer = (unsigned char *)iov[iov_count].iov_base;
-        bConverted = remaining; /* how much will get unpacked this time */
-        user_memory = pConv->pBaseBuf + initial_displ + stack[0].disp + stack[1].disp;
-        i = pConv->count - stack[0].count;  /* how many we already packed */
-        assert(i == (pConv->bConverted / pData->size));
-
-        if( packed_buffer == NULL ) {
-            /* special case for small data. We avoid allocating memory if we
-             * can fill the iovec directly with the address of the remaining
-             * data.
-             */
-            if( stack->count < (size_t)((*out_size) - iov_count) ) {
-                stack[1].count = pData->size - (pConv->bConverted % pData->size);
-                for( index = iov_count; i < pConv->count; i++, index++ ) {
-                    iov[index].iov_base = (IOVBASE_TYPE *) user_memory;
-                    iov[index].iov_len = stack[1].count;
-                    stack[0].disp += extent;
-                    pConv->bConverted += stack[1].count;
-                    stack[1].disp  = 0;  /* reset it for the next round */
-                    stack[1].count = pData->size;
-                    user_memory = pConv->pBaseBuf + initial_displ + stack[0].disp;
-                    COMPUTE_CSUM( iov[index].iov_base, iov[index].iov_len, pConv );
-                }
-                *out_size = iov_count + index;
-                *max_data = (pConv->bConverted - initial_bytes_converted);
-                pConv->flags |= CONVERTOR_COMPLETED;
-                return 1;  /* we're done */
-            }
-            /* now special case for big contiguous data with gaps around */
-            if( pData->size >= IOVEC_MEM_LIMIT ) {
-                /* as we dont have to copy any data, we can simply fill the iovecs
-                 * with data from the user data description.
-                 */
-                for( index = iov_count; (i < pConv->count) && (index < (*out_size));
-                     i++, index++ ) {
-                    if( remaining < pData->size ) {
-                        iov[index].iov_base = (IOVBASE_TYPE *) user_memory;
-                        iov[index].iov_len = remaining;
-                        remaining = 0;
-                        COMPUTE_CSUM( iov[index].iov_base, iov[index].iov_len, pConv );
-                        break;
-                    } else {
-                        iov[index].iov_base = (IOVBASE_TYPE *) user_memory;
-                        iov[index].iov_len = pData->size;
-                        user_memory += extent;
-                        COMPUTE_CSUM( iov[index].iov_base, (size_t)iov[index].iov_len, pConv );
-                    }
-                    remaining -= iov[index].iov_len;
-                    pConv->bConverted += iov[index].iov_len;
-                }
-                *out_size = index;
-                *max_data = (pConv->bConverted - initial_bytes_converted);
-                if( pConv->bConverted == pConv->local_size ) {
-                    pConv->flags |= CONVERTOR_COMPLETED;
-                    return 1;
-                }
-                return 0;
+        if( remaining > iov[idx].iov_len )
+            remaining = iov[idx].iov_len;
+        packed_buffer = (unsigned char *)iov[idx].iov_base;
+        pConv->bConverted += remaining;
+        user_memory = pConv->pBaseBuf + pData->true_lb + stack[0].disp + stack[1].disp;
+
+        DO_DEBUG( opal_output( 0, "pack_homogeneous_contig( user_memory %p, packed_buffer %p length %" PRIsize_t "\n",
+                               (void*)user_memory, (void*)packed_buffer, remaining ); );
+
+        length = (0 == pConv->stack_pos ? 0 : stack[1].count);  /* left over from the last pack */
+        /* data left from last round and enough space in the buffer */
+        if( (pData->size != length) && (length <= remaining)) {
+            /* copy the partial left-over from the previous round */
+            OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, length, pConv->pBaseBuf,
+                                             pData, pConv->count );
+            DO_DEBUG( opal_output( 0, "pack dest %p src %p length %" PRIsize_t " [prologue]\n",
+                                   (void*)user_memory, (void*)packed_buffer, length ); );
+            MEMCPY_CSUM( packed_buffer, user_memory, length, pConv );
+            packed_buffer  += length;
+            remaining      -= length;
+            stack[1].count -= length;
+            stack[1].disp  += length;  /* just in case, we overwrite this below */
+            if( 0 == stack[1].count) { /* one completed element */
+                stack[0].count--;
+                stack[0].disp += extent;
+                if( 0 == stack[0].count )  /* not yet done */
+                    break;
+                stack[1].count = pData->size;
+                stack[1].disp = 0;
             }
+            user_memory = pConv->pBaseBuf + pData->true_lb + stack[0].disp + stack[1].disp;
         }
 
-        {
-            DO_DEBUG( opal_output( 0, "pack_homogeneous_contig( user_memory %p, packed_buffer %p length %lu\n",
-                                   (void*)user_memory, (void*)packed_buffer, (unsigned long)remaining ); );
-
-            length = (0 == pConv->stack_pos ? 0 : stack[1].count);  /* left over from the last pack */
-            /* data left from last round and enough space in the buffer */
-            if( (0 != length) && (length <= remaining)) {
-                /* copy the partial left-over from the previous round */
-                OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, length, pConv->pBaseBuf,
-                                                 pData, pConv->count );
-                DO_DEBUG( opal_output( 0, "2. pack dest %p src %p length %lu\n",
-                                       (void*)user_memory, (void*)packed_buffer, (unsigned long)length ); );
-                MEMCPY_CSUM( packed_buffer, user_memory, length, pConv );
-                packed_buffer  += length;
-                user_memory    += (extent - pData->size + length);
-                remaining      -= length;
-                stack[1].count -= length;
-                if( 0 == stack[1].count) { /* one completed element */
-                    stack[0].count--;
-                    stack[0].disp += extent;
-                    if( 0 != stack[0].count ) {  /* not yet done */
-                        stack[1].count = pData->size;
-                        stack[1].disp = 0;
-                    }
-                }
-            }
-            for( i = 0;  pData->size <= remaining; i++ ) {
-                OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, pData->size, pConv->pBaseBuf,
-                                                 pData, pConv->count );
-                DO_DEBUG( opal_output( 0, "3. pack dest %p src %p length %lu\n",
-                                       (void*)user_memory, (void*)packed_buffer, (unsigned long)pData->size ); );
-                MEMCPY_CSUM( packed_buffer, user_memory, pData->size, pConv );
-                packed_buffer += pData->size;
-                user_memory   += extent;
-                remaining   -= pData->size;
-            }
-            stack[0].count -= i;  /* the filled up and the entire types */
-            stack[0].disp  += (i * extent);
-            stack[1].disp  += remaining;
-            /* Copy the last bits */
-            if( 0 != remaining ) {
-                OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, remaining, pConv->pBaseBuf,
-                                                 pData, pConv->count );
-                DO_DEBUG( opal_output( 0, "4. pack dest %p src %p length %lu\n",
-                                       (void*)user_memory, (void*)packed_buffer, (unsigned long)remaining ); );
-                MEMCPY_CSUM( packed_buffer, user_memory, remaining, pConv );
-                user_memory += remaining;
-                stack[1].count -= remaining;
-            }
+        for( i = 0; pData->size <= remaining; i++ ) {
+            OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, pData->size, pConv->pBaseBuf,
+                                             pData, pConv->count );
+            DO_DEBUG( opal_output( 0, "pack dest %p src %p length %" PRIsize_t " [%" PRIsize_t "/%" PRIsize_t "\n",
+                                   (void*)user_memory, (void*)packed_buffer, pData->size, remaining, iov[idx].iov_len ); );
+            MEMCPY_CSUM( packed_buffer, user_memory, pData->size, pConv );
+            packed_buffer += pData->size;
+            user_memory   += extent;
+            remaining     -= pData->size;
+        }
+        stack[0].count -= i;  /* the entire datatype copied above */
+        stack[0].disp  += (i * extent);
+
+        /* Copy the last bits */
+        if( 0 != remaining ) {
+            OPAL_DATATYPE_SAFEGUARD_POINTER( user_memory, remaining, pConv->pBaseBuf,
+                                             pData, pConv->count );
+            DO_DEBUG( opal_output( 0, "4. pack dest %p src %p length %" PRIsize_t "\n",
+                                   (void*)user_memory, (void*)packed_buffer, remaining ); );
+            MEMCPY_CSUM( packed_buffer, user_memory, remaining, pConv );
+            stack[1].count -= remaining;
+            stack[1].disp  += remaining;  /* keep the += in case we are copying less that the datatype size */
             if( 0 == stack[1].count ) {  /* prepare for the next element */
                 stack[1].count = pData->size;
                 stack[1].disp  = 0;
             }
         }
-        pConv->bConverted += bConverted;
     }
-    *out_size = iov_count;
-    *max_data = (pConv->bConverted - initial_bytes_converted);
-    if( pConv->bConverted == pConv->local_size ) {
-        pConv->flags |= CONVERTOR_COMPLETED;
-        return 1;
-    }
-    return 0;
+
+ update_status_and_return:
+    *out_size = idx;
+    *max_data = pConv->bConverted - initial_bytes_converted;
+    if( pConv->bConverted == pConv->local_size ) pConv->flags |= CONVERTOR_COMPLETED;
+    return !!(pConv->flags & CONVERTOR_COMPLETED);  /* done or not */
 }
 
 /* The pack/unpack functions need a cleanup. I have to create a proper interface to access