unposted: reformulate 32285 to lift the fheap->sp test out of the loop, improve commentary

2 files changed, 14 insertions, 15 deletions

diff --git a/ChangeLog b/ChangeLog
index 4a134644d..ac5759e46 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2014-01-22  Barton E. Schaefer  <schaefer@zsh.org>
+
+	* unposted: Src/mem.c: reformulate 32285 to lift the fheap->sp
+	test out of the loop, improve commentary
+
 2014-01-19  Barton E. Schaefer  <schaefer@zsh.org>
 
 	* 32294 (plus typo fix): Src/utils.c: prevent buffer overflow when
diff --git a/Src/mem.c b/Src/mem.c
index d15721c28..a7f11a62b 100644
--- a/Src/mem.c
+++ b/Src/mem.c
@@ -319,23 +319,26 @@ freeheap(void)
     h_free++;
 #endif
 
-    /* At this point we used to do:
-    fheap = NULL;
-     *
+    /*
      * When pushheap() is called, it sweeps over the entire heaps list of
      * arenas and marks every one of them with the amount of free space in
      * that arena at that moment.  zhalloc() is then allowed to grab bits
      * out of any of those arenas that have free space.
      *
-     * With the above reset of fheap, the loop below sweeps back over the
+     * Whenever fheap is NULL here, the loop below sweeps back over the
      * entire heap list again, resetting the free space in every arena to
      * the amount stashed by pushheap() and finding the first arena with
      * free space to optimize zhalloc()'s next search.  When there's a lot
      * of stuff already on the heap, this is an enormous amount of work,
      * and performance goes to hell.
      *
-     * However, there doesn't seem to be any reason to reset fheap before
-     * beginning this loop.  Either it's already correct, or it has never
+     * However, if the arena to which fheap points is unused, we want to
+     * free it, so we have no choice but to do the sweep for a new fheap.
+     */
+    if (fheap && !fheap->sp)
+	fheap = NULL;	/* We used to do this unconditionally */
+    /*
+     * In other cases, either fheap is already correct, or it has never
      * been set and this loop will do it, or it'll be reset from scratch
      * on the next popheap().  So all that's needed here is to pick up
      * the scan wherever the last pass [or the last popheap()] left off.
@@ -367,15 +370,6 @@ freeheap(void)
 	    }
 #endif
 	} else {
-	    if (h == fheap && h != heaps) {
-		/*
-		 * When deallocating the last arena with free space,
-		 * loop back through the list to find another one.
-		 */
-		fheap = NULL;
-		hn = heaps;
-		continue;
-	    }
 #ifdef USE_MMAP
 	    munmap((void *) h, h->size);
 #else