Add some comments on the internals of the bucket structure. Alter the cleanup

function to make it more scalable. The memory fragmentation is still high, but
at least in most of the cases (where all ressources are correctly released
before the cleanup) the code is now highly efficient. Before the code execute
in (N * (N-1))!, which take a while when the number of allocated ressources
increase (which is the case when a lot of unexpected messages are created).

The fix consist of checking if all items are freed and if it's the case
then do not recreate the free items list (as we know that everything will
be released). If this condition is not true, we fall back on the
original execution path (which is still sub-sub-sub ... optimal).

This commit was SVN r14406.

ompi/mca/allocator/bucket/allocator_bucket_alloc.c

@@ -308,44 +308,81 @@ int mca_allocator_bucket_cleanup(mca_allocator_base_module_t * mem)
     for(i = 0; i < mem_options->num_buckets; i++) {
         OPAL_THREAD_LOCK(&(mem_options->buckets[i].lock));
         segment_header = &(mem_options->buckets[i].segment_head);
-        /* traverse the list of segment headers until we hit NULL */
-        while(NULL != *segment_header) {
-            first_chunk = (*segment_header)->first_chunk; 
+        if( NULL == (*segment_header) ) {
+            OPAL_THREAD_UNLOCK(&(mem_options->buckets[i].lock));
+            continue;
+        }
+        /* first we suppose the execution is correct and all chunks
+         * have ben correctly released. Therefore, if we make sure
+         * all segments only contain free items then we can release
+         * everything in one go.
+         */
+        empty = true;
+        segment = mem_options->buckets[i].segment_head;
+        while( (true != empty) || (NULL != segment) ) {
+            first_chunk = segment->first_chunk; 
             chunk = first_chunk;
             /* determine if the segment is free */
-            do
-            {
+            do {
                 if(chunk->u.bucket == i) {
                     empty = false;
+                    break;
                 }
                 chunk = chunk->next_in_segment;
-            } while(empty && (chunk != first_chunk));
-            if(empty) {
-                chunk = first_chunk;
-                /* remove the chunks from the free list */
-                do
-                {
-                    if(mem_options->buckets[i].free_chunk == chunk) {
-                        mem_options->buckets[i].free_chunk = chunk->u.next_free;
-                    } else {
-                        next_chunk = mem_options->buckets[i].free_chunk;
-                        while(next_chunk->u.next_free != chunk) {
-                            next_chunk = next_chunk->u.next_free;
-                        }
-                        next_chunk->u.next_free = chunk->u.next_free; 
-                    }
-                } while((chunk = chunk->next_in_segment) != first_chunk);
-                /* set the segment list to point to the next segment */
-                segment = *segment_header;
-                *segment_header = segment->next_segment;
+            } while(chunk != first_chunk);
+            /* go to next segment */
+            segment = segment->next_segment;
+        }
+        if( true == empty ) {  /* all segments ready for release */
+            mca_allocator_bucket_segment_head_t* next_segment;
+            segment = mem_options->buckets[i].segment_head;
+            while( NULL != segment ) {
+                next_segment = segment->next_segment;
                 /* free the memory */
                 if(mem_options->free_mem_fn)
                     mem_options->free_mem_fn(mem->alc_mpool, segment);
-            } else {
-                /* go to next segment */
-                segment_header = &((*segment_header)->next_segment);
+                segment = next_segment;
+            }
+            mem_options->buckets[i].free_chunk = NULL;
+            mem_options->buckets[i].segment_head = NULL;
+        } else {
+            /* traverse the list of segment headers until we hit NULL */
+            while(NULL != *segment_header) {
+                first_chunk = (*segment_header)->first_chunk; 
+                chunk = first_chunk;
+                empty = true;
+                /* determine if the segment is free */
+                do {
+                    if(chunk->u.bucket == i) {
+                        empty = false;
+                    }
+                    chunk = chunk->next_in_segment;
+                } while(empty && (chunk != first_chunk));
+                if(empty) {
+                    chunk = first_chunk;
+                    /* remove the chunks from the free list */
+                    do {
+                        if(mem_options->buckets[i].free_chunk == chunk) {
+                            mem_options->buckets[i].free_chunk = chunk->u.next_free;
+                        } else {
+                            next_chunk = mem_options->buckets[i].free_chunk;
+                            while(next_chunk->u.next_free != chunk) {
+                                next_chunk = next_chunk->u.next_free;
+                            }
+                            next_chunk->u.next_free = chunk->u.next_free; 
+                        }
+                    } while((chunk = chunk->next_in_segment) != first_chunk);
+                    /* set the segment list to point to the next segment */
+                    segment = *segment_header;
+                    *segment_header = segment->next_segment;
+                    /* free the memory */
+                    if(mem_options->free_mem_fn)
+                        mem_options->free_mem_fn(mem->alc_mpool, segment);
+                } else {
+                    /* go to next segment */
+                    segment_header = &((*segment_header)->next_segment);
+                }
             }
-            empty = true; 
         }
         /* relese the lock on the bucket */
         OPAL_THREAD_UNLOCK(&(mem_options->buckets[i].lock));

ompi/mca/allocator/bucket/allocator_bucket_alloc.h

@@ -40,7 +40,13 @@ struct mca_allocator_bucket_chunk_header_t {
                /**< The next chunk in the memory segment */
     /**
       * Union which holds either a pointer to the next free chunk
-      * or the bucket number
+      * or the bucket number. Based on the current location of the chunk
+      * we use one or the other of these fields. If the chunk is owned
+      * by the user, then the bucket field is set, which allow us to know
+      * in which specific bucket we have to put it back on free (as the
+      * chunk don't have the size attached). When the allocator own the
+      * chunk, the next_free fild is used, which allow us to put these
+      * chunks a list of free elements.
       */
     union u {
         struct mca_allocator_bucket_chunk_header_t * next_free;