Added tests for heap_finish_collection and cleaned up the code a bit

diff --git a/build.rb b/build.rb
index f2c923ba44f9fe2bd9303db8b14dd832d4a77faf..14b1dd309297e22c70b00e11b623295cdf1f478a 100755 (executable)
--- a/build.rb
+++ b/build.rb
@@ -26,7 +26,6 @@ base_env = BuildEnv.new do |env|
     env['CFLAGS'] += ['-pg']
     env['LDFLAGS'] += ['-pg']
   end
-
   # Enable coverage info
   if Opts[:profile].include? "coverage"
     env['CFLAGS'] += ['--coverage']

diff --git a/source/runtime/heap.c b/source/runtime/heap.c
index ee2a34fc0fbdbf591ece3d2bf4f56494e6aaf583..22e710f8f95bc52056800cd6d2253d4f9778ff4a 100644 (file)
--- a/source/runtime/heap.c
+++ b/source/runtime/heap.c
@@ -103,32 +103,28 @@ void heap_finish_collection(heap_t* heap)
 {
     /* All blocks remaining in the greylist now are unreachable so free them */
     splaytree_destroy(heap->greylist);
+    heap->greylist = NULL;
     /* Now iterate over the sub heaps and make sure the full/available lists are correct */
     for (unsigned int i = 0; i < NUM_HEAP_STACKS; i++) {
-        segment_t* prev = NULL;
-        segment_t* curr = heap->heaps[i].full;
+        segment_t* avail = heap->heaps[i].available;
+        segment_t* full  = NULL;
+        segment_t* curr  = heap->heaps[i].full;
         while(curr != NULL) {
-            if (segment_empty(curr)) {
-                segment_t* deadite = curr;
-                if (NULL != prev) {
-                    prev->next = curr->next;
-                    curr = curr->next;
-                }
-                deadite->next = NULL;
-                segment_destroy(deadite);
-            } else if (!segment_full(curr)) {
-                if (NULL != prev) {
-                    prev->next = curr->next;
-                    curr = curr->next;
-                }
-                curr->next = heap->heaps[i].available;
-                heap->heaps[i].available = curr;
+            segment_t* seg = curr;
+            curr = curr->next;
+            if (segment_full(seg)) {
+                seg->next = full;
+                full = seg;
+            } else if (segment_empty(seg)) {
+                seg->next = NULL;
+                segment_destroy(seg);
             } else {
-                prev = curr;
-                curr = curr->next;
+                seg->next = avail;
+                avail = seg;
             }
         }
-        segment_destroy(heap->heaps[i].available);
+        heap->heaps[i].available = avail;
+        heap->heaps[i].full = full;
     }
 }
 

diff --git a/tests/test_heap.c b/tests/test_heap.c
index 5c24af91e20fb66e96655d0fd17d05bd5d0ff6fb..e1a9cde2fc3db22a2abc231229efc17203e43150 100644 (file)
--- a/tests/test_heap.c
+++ b/tests/test_heap.c
@@ -69,4 +69,76 @@ TEST_SUITE(Heap) {
         CHECK(blocks != heap->blocks);
         heap_destroy(heap);
     }
+
+    TEST(Verify_finish_collection_moves_segments_from_the_full_list_to_the_available_list_when_some_blocks_are_reclaimed) {
+        heap_t* heap = heap_create();
+        segment_t* seg = segment_create(2, NULL);
+        seg->blockmap[0] = 42;
+        splaytree_t* blocks = heap->blocks;
+        heap->heaps[1].full = seg;
+        heap_start_collection(heap);
+        heap_finish_collection(heap);
+        CHECK(blocks != heap->blocks);
+        CHECK(heap->heaps[1].available == seg);
+        CHECK(heap->heaps[1].full == NULL);
+        heap_destroy(heap);
+    }
+
+    TEST(Verify_finish_collection_moves_segments_from_the_full_list_to_the_available_list_when_some_blocks_are_reclaimed) {
+        heap_t* heap = heap_create();
+        segment_t* seg2 = segment_create(2, NULL);
+        segment_t* seg1 = segment_create(2, seg2);
+        seg1->blockmap[0] = 0;
+        seg2->blockmap[0] = 42;
+        splaytree_t* blocks = heap->blocks;
+        heap->heaps[1].full = seg1;
+        heap_start_collection(heap);
+        heap_finish_collection(heap);
+        CHECK(blocks != heap->blocks);
+        CHECK(heap->heaps[1].available == seg2);
+        CHECK(heap->heaps[1].full == seg1);
+        heap_destroy(heap);
+    }
+
+    TEST(Verify_finish_collection_frees_empty_segments_in_the_full_list) {
+        heap_t* heap = heap_create();
+        segment_t* seg = segment_create(2, NULL);
+        splaytree_t* blocks = heap->blocks;
+        heap->heaps[1].full = seg;
+        heap_start_collection(heap);
+        heap_finish_collection(heap);
+        CHECK(blocks != heap->blocks);
+        CHECK(heap->heaps[1].available == NULL);
+        CHECK(heap->heaps[1].full == NULL);
+        heap_destroy(heap);
+    }
+
+    TEST(Verify_finish_collection_frees_empty_segments_in_the_full_list) {
+        heap_t* heap = heap_create();
+        segment_t* seg2 = segment_create(2, NULL);
+        segment_t* seg1 = segment_create(2, seg2);
+        seg1->blockmap[0] = 0;
+        splaytree_t* blocks = heap->blocks;
+        heap->heaps[1].full = seg1;
+        heap_start_collection(heap);
+        heap_finish_collection(heap);
+        CHECK(blocks != heap->blocks);
+        CHECK(heap->heaps[1].available == NULL);
+        CHECK(heap->heaps[1].full == seg1);
+        heap_destroy(heap);
+    }
+
+    TEST(Verify_finish_collection_does_nothing_when_full_list_contains_only_full_segments) {
+        heap_t* heap = heap_create();
+        segment_t* seg = segment_create(2, NULL);
+        seg->blockmap[0] = 0;
+        splaytree_t* blocks = heap->blocks;
+        heap->heaps[1].full = seg;
+        heap_start_collection(heap);
+        heap_finish_collection(heap);
+        CHECK(blocks != heap->blocks);
+        CHECK(heap->heaps[1].available == NULL);
+        CHECK(heap->heaps[1].full == seg);
+        heap_destroy(heap);
+    }
 }