[svn-r5126] Purpose:

    Bug Fix (sorta)
Description:
    "small" contiguous hyperslabs were not being detected correctly by
    H5S_hyper_select_contigous() leading to poorer performance than possible.
Solution:
    Corrected check for small hyperslabs.  Also cleaned up the H5S_find() code.
Platforms tested:
    FreeBSD 4.5 (sleipnir)

2 files changed, 105 insertions, 43 deletions

diff --git a/src/H5S.c b/src/H5S.c
index 877de5d..0b6632c 100644
--- a/src/H5S.c
+++ b/src/H5S.c
@@ -1496,9 +1496,9 @@ H5S_set_extent_simple (H5S_t *space, unsigned rank, const hsize_t *dims,
 H5S_conv_t *
 H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
 {
-    size_t	i;
-    htri_t c1,c2;
-    H5S_conv_t	*path;
+    H5S_conv_t	*path;  /* Space conversion path */
+    htri_t c1,c2;       /* Flags whether a selection is contiguous */
+    size_t	i;      /* Index variable */
     
     FUNC_ENTER (H5S_find, NULL);
 
@@ -1512,11 +1512,8 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
      * We can't do conversion if the source and destination select a
      * different number of data points.
      */
-    if (H5S_get_select_npoints(mem_space) !=
-	H5S_get_select_npoints (file_space)) {
-        HRETURN_ERROR (H5E_DATASPACE, H5E_BADRANGE, NULL,
-		       "memory and file data spaces are different sizes");
-    }
+    if (H5S_get_select_npoints(mem_space) != H5S_get_select_npoints (file_space))
+        HRETURN_ERROR (H5E_DATASPACE, H5E_BADRANGE, NULL, "memory and file data spaces are different sizes");
 
     /*
      * Is this path already present in the data space conversion path table?
@@ -1524,15 +1521,14 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
      */
     for (i=0; i<H5S_nconv_g; i++) {
         if (H5S_conv_g[i]->f->type==file_space->select.type &&
-            H5S_conv_g[i]->m->type==mem_space->select.type) {
+                H5S_conv_g[i]->m->type==mem_space->select.type) {
             /*
              * Initialize direct read/write functions
              */
             c1=H5S_select_contiguous(file_space);
             c2=H5S_select_contiguous(mem_space);
             if(c1==FAIL || c2==FAIL)
-                HRETURN_ERROR(H5E_DATASPACE, H5E_BADRANGE, NULL,
-                      "invalid check for contiguous dataspace ");
+                HRETURN_ERROR(H5E_DATASPACE, H5E_BADRANGE, NULL, "invalid check for contiguous dataspace ");
 
             if (c1==TRUE && c2==TRUE) {
                 H5S_conv_g[i]->read = H5S_all_read;
@@ -1551,20 +1547,16 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
      * The path wasn't found.  Do we have enough information to create a new
      * path?
      */
-    if (NULL==H5S_fconv_g[file_space->select.type] ||
-            NULL==H5S_mconv_g[mem_space->select.type]) {
-        HRETURN_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, NULL,
-		      "unable to convert between data space selections");
-    }
+    if (NULL==H5S_fconv_g[file_space->select.type] || NULL==H5S_mconv_g[mem_space->select.type])
+        HRETURN_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, NULL, "unable to convert between data space selections");
 
     /*
      * Create a new path.
      */
-    if (NULL==(path = H5MM_calloc(sizeof(*path)))) {
-        HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL,
-		      "memory allocation failed for data space conversion "
-		      "path");
-    }
+    if (NULL==(path = H5MM_calloc(sizeof(*path))))
+        HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for data space conversion path");
+
+    /* Initialize file & memory conversion functions */
     path->f = H5S_fconv_g[file_space->select.type];
     path->m = H5S_mconv_g[mem_space->select.type];
 
@@ -1574,13 +1566,12 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
     c1=H5S_select_contiguous(file_space);
     c2=H5S_select_contiguous(mem_space);
     if(c1==FAIL || c2==FAIL)
-        HRETURN_ERROR(H5E_DATASPACE, H5E_BADRANGE, NULL,
-		      "invalid check for contiguous dataspace ");
+        HRETURN_ERROR(H5E_DATASPACE, H5E_BADRANGE, NULL, "invalid check for contiguous dataspace ");
 
     if (c1==TRUE && c2==TRUE) {
         path->read = H5S_all_read;
         path->write = H5S_all_write;
-    }
+    } /* end if */
     
     /*
      * Add the new path to the table.
@@ -1589,14 +1580,11 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space)
         size_t n = MAX(10, 2*H5S_aconv_g);
         H5S_conv_t **p = H5MM_realloc(H5S_conv_g, n*sizeof(H5S_conv_g[0]));
 
-        if (NULL==p) {
-            HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL,
-                  "memory allocation failed for data space conversion "
-                  "path table");
-        }
+        if (NULL==p)
+            HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for data space conversion path table");
         H5S_aconv_g = n;
         H5S_conv_g = p;
-    }
+    } /* end if */
     H5S_conv_g[H5S_nconv_g++] = path;
 
     FUNC_LEAVE(path);
diff --git a/src/H5Shyper.c b/src/H5Shyper.c
index 54738ff..c6639d7 100644
--- a/src/H5Shyper.c
+++ b/src/H5Shyper.c
@@ -5780,37 +5780,70 @@ H5S_hyper_bounds(H5S_t *space, hsize_t *start, hsize_t *end)
 htri_t
 H5S_hyper_select_contiguous(const H5S_t *space)
 {
-    htri_t ret_value=FAIL;  /* return value */
     H5S_hyper_span_info_t *spans;   /* Hyperslab span info node */
-    H5S_hyper_span_t *span;     /* Hyperslab span node */
-    unsigned u;                    /* index variable */
+    H5S_hyper_span_t *span;         /* Hyperslab span node */
+    unsigned u;                     /* index variable */
+    unsigned small_contiguous,      /* Flag for small contiguous block */
+        large_contiguous;           /* Flag for large contiguous block */
+    htri_t ret_value=FALSE;         /* return value */
 
     FUNC_ENTER (H5S_hyper_select_contiguous, FAIL);
 
     assert(space);
 
-    /* Quicker check for a "regular" hyperslab selection */
+    /* Check for a "regular" hyperslab selection */
     if(space->select.sel_info.hslab.diminfo != NULL) {
         /*
          * For a regular hyperslab to be contiguous, it must have only one
          * block (i.e. count==1 in all dimensions) and the block size must be
          * the same as the dataspace extent's in all but the slowest changing
-         * dimension.
+         * dimension. (dubbed "large contiguous" block)
+         *
+         * OR
+         *
+         * The selection must have only one block (i.e. count==1) and the block
+         * size must be 1 in all but the fastest changing dimension. (dubbed
+         * "small contiguous" block)
          */
-        ret_value=TRUE;	/* assume true and reset if the dimensions don't match */
+
+        /* Initialize flags */
+        large_contiguous=TRUE;	/* assume true and reset if the dimensions don't match */
+        small_contiguous=FALSE;	/* assume false initially */
+
+        /* Check for a "large contigous" block */
         for(u=1; u<space->extent.u.simple.rank; u++) {
             if(space->select.sel_info.hslab.diminfo[u].count>1 || space->select.sel_info.hslab.diminfo[u].block!=space->extent.u.simple.size[u]) {
-                ret_value=FALSE;
+                large_contiguous=FALSE;
                 break;
             } /* end if */
         } /* end for */
+
+        /* If we didn't find a large contiguous block, check for a small one */
+        if(large_contiguous==FALSE) {
+            small_contiguous=TRUE;
+            for(u=0; u<(space->extent.u.simple.rank-1); u++) {
+                if(space->select.sel_info.hslab.diminfo[u].count>1 || space->select.sel_info.hslab.diminfo[u].block!=1) {
+                    small_contiguous=FALSE;
+                    break;
+                } /* end if */
+            } /* end for */
+        } /* end if */
+
+        /* Indicate true if it's either a large or small contiguous block */
+        if(large_contiguous || small_contiguous)
+            ret_value=TRUE;
     } /* end if */
     else {
         /*
-         * For a hyperslab to be contiguous, it's size must be the same as the
-         * dataspace extent's in all but the slowest changing dimension
+         * For a hyperslab to be contiguous, it must have only one block and
+         * (either it's size must be the same as the dataspace extent's in all
+         * but the slowest changing dimension
+         * OR
+         * block size must be 1 in all but the fastest changing dimension).
          */
-        ret_value=TRUE;	/* assume true and reset if the dimensions don't match */
+        /* Initialize flags */
+        large_contiguous=TRUE;	/* assume true and reset if the dimensions don't match */
+        small_contiguous=FALSE;	/* assume false initially */
 
         /* Get information for slowest changing information */
         spans=space->select.sel_info.hslab.span_lst;
@@ -5818,7 +5851,7 @@ H5S_hyper_select_contiguous(const H5S_t *space)
 
         /* If there are multiple spans in the slowest changing dimension, the selection isn't contiguous */
         if(span->next!=NULL)
-            ret_value=FALSE;
+            large_contiguous=FALSE;
         else {
             /* Now check the rest of the dimensions */
             if(span->down!=NULL) {
@@ -5833,13 +5866,13 @@ H5S_hyper_select_contiguous(const H5S_t *space)
 
                     /* Check that this is the only span and it spans the entire dimension */
                     if(span->next!=NULL) {
-                        ret_value=FALSE;
+                        large_contiguous=FALSE;
                         break;
                     } /* end if */
                     else {
                         /* If this span doesn't cover the entire dimension, then this selection isn't contiguous */
                         if(((span->high-span->low)+1)!=(hssize_t)space->extent.u.simple.size[u]) {
-                            ret_value=FALSE;
+                            large_contiguous=FALSE;
                             break;
                         } /* end if */
                         else {
@@ -5853,6 +5886,47 @@ H5S_hyper_select_contiguous(const H5S_t *space)
                 } /* end while */
             } /* end if */
         } /* end else */
+
+        /* If we didn't find a large contiguous block, check for a small one */
+        if(large_contiguous==FALSE) {
+            small_contiguous=TRUE;
+
+            /* Get information for slowest changing information */
+            spans=space->select.sel_info.hslab.span_lst;
+            span=spans->head;
+
+            /* Current dimension working on */
+            u=0;
+
+            /* Cycle down the spans until we run out of down spans or find a non-contiguous span */
+            while(spans!=NULL) {
+                span=spans->head;
+
+                /* Check that this is the only span and it spans the entire dimension */
+                if(span->next!=NULL) {
+                    small_contiguous=FALSE;
+                    break;
+                } /* end if */
+                else {
+                    /* If this span doesn't cover the entire dimension, then this selection isn't contiguous */
+                    if(u<(space->extent.u.simple.rank-1) && ((span->high-span->low)+1)!=1) {
+                        small_contiguous=FALSE;
+                        break;
+                    } /* end if */
+                    else {
+                        /* Walk down to the next span */
+                        spans=span->down;
+
+                        /* Increment dimension */
+                        u++;
+                    } /* end else */
+                } /* end else */
+            } /* end while */
+        } /* end if */
+
+        /* Indicate true if it's either a large or small contiguous block */
+        if(large_contiguous || small_contiguous)
+            ret_value=TRUE;
     } /* end else */
 
     FUNC_LEAVE (ret_value);