zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: zsh-workers@zsh.org
Subject: Re: Slowdown around 5.0.5-dev-0
Date: Sun, 11 Oct 2015 09:17:57 -0700	[thread overview]
Message-ID: <151011091757.ZM27755@torch.brasslantern.com> (raw)
In-Reply-To: <CAKc7PVD8dU29eyfF-QJPUicFGUT6Kpomu+qZNungwnXpbjCH7Q@mail.gmail.com>

On Oct 11, 10:39am, Sebastian Gniazdowski wrote:
}
} Zprof says there is no change:

Well, that's not entirely surprising.  It probably just means that there
is some space in a lot of arenas, so fpop doesn't move far from the head
of the list.  Or it could mean that the real problem is popheap() rather
than freeheap().

} BTW, the patch didn't apply to head or to 23f98c3,

Curious.  "git diff master origin/master" shows no diffs for my local
repository, and the patch was just a straight "git diff".

There were additional changes committed after 23f98c3 so there's no
reason to have expected it would apply to that.

In any case here's another stab at it.  Should apply to 83a1757.  This
should help if there's a lot of freeheap() happening, but not so much
if there's a lot of push/pop (in fact it may be worse in that case).

If it's push/pop that's the issue, the next step would be to try to
identify the crucial pushheap() and replace it with NEWHEAPS() if the
scope permits.


diff --git a/Src/mem.c b/Src/mem.c
index b9569ea..d49f685 100644
--- a/Src/mem.c
+++ b/Src/mem.c
@@ -294,7 +294,7 @@ pushheap(void)
 #endif
 
     for (h = heaps; h; h = h->next) {
-	DPUTS(!h->used, "BUG: empty heap");
+	DPUTS(!h->used && h->next, "BUG: empty heap");
 	hs = (Heapstack) zalloc(sizeof(*hs));
 	hs->next = h->sp;
 	h->sp = hs;
@@ -334,17 +334,15 @@ freeheap(void)
      *
      * 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
+     * the amount stashed by pushheap() and finding the arena with the most
      * 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, 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 */
-    /*
+     * Therefore, we defer freeing the most recently allocated arena until
+     * we reach popheap().  This may fail to reclaim some space in earlier
+     * arenas.
+     *
      * 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
@@ -361,7 +359,11 @@ freeheap(void)
 	    memset(arena(h) + h->sp->used, 0xff, h->used - h->sp->used);
 #endif
 	    h->used = h->sp->used;
-	    if (!fheap && h->used < ARENA_SIZEOF(h))
+	    if (!fheap) {
+		if (h->used < ARENA_SIZEOF(h))
+		    fheap = h;
+	    } else if (ARENA_SIZEOF(h) - h->used >
+		       ARENA_SIZEOF(fheap) - fheap->used)
 		fheap = h;
 	    hl = h;
 #ifdef ZSH_HEAP_DEBUG
@@ -384,6 +386,26 @@ freeheap(void)
 	    VALGRIND_MEMPOOL_TRIM((char *)h, (char *)arena(h), h->used);
 #endif
 	} else {
+	    if (h->next) {
+		/* We want to cut this out of the arena list if we can */
+		if (h == heaps)
+		    hl = heaps = h->next;
+		else if (hl && hl->next == h)
+		    hl->next = h->next;
+		else {
+		    DPUTS(hl, "hl->next != h when freeing");
+		    hl = h;
+		    continue;
+		}
+		h->next = NULL;
+	    } else {
+		/* Leave an empty arena at the end until popped */
+		h->used = 0;
+		fheap = hl = h;
+		break;
+	    }
+	    if (fheap == h)
+		fheap = NULL;
 #ifdef USE_MMAP
 	    munmap((void *) h, h->size);
 #else
@@ -441,12 +463,29 @@ popheap(void)
 #ifdef ZSH_VALGRIND
 	    VALGRIND_MEMPOOL_TRIM((char *)h, (char *)arena(h), h->used);
 #endif
-	    if (!fheap && h->used < ARENA_SIZEOF(h))
+	    if (!fheap) {
+		if (h->used < ARENA_SIZEOF(h))
+		    fheap = h;
+	    } else if (ARENA_SIZEOF(h) - h->used >
+		       ARENA_SIZEOF(fheap) - fheap->used)
 		fheap = h;
 	    zfree(hs, sizeof(*hs));
 
 	    hl = h;
 	} else {
+	    if (h->next) {
+		/* We want to cut this out of the arena list if we can */
+		if (h == heaps)
+		    hl = heaps = h->next;
+		else if (hl && hl->next == h)
+		    hl->next = h->next;
+		else {
+		    DPUTS(hl, "hl->next != h when popping");
+		    hl = h;
+		    continue;
+		}
+		h->next = NULL;
+	    }
 #ifdef USE_MMAP
 	    munmap((void *) h, h->size);
 #else
@@ -524,7 +563,7 @@ zheapptr(void *p)
 mod_export void *
 zhalloc(size_t size)
 {
-    Heap h;
+    Heap h, hp = NULL;
     size_t n;
 #ifdef ZSH_VALGRIND
     size_t req_size = size;
@@ -546,6 +585,7 @@ zhalloc(size_t size)
     for (h = ((fheap && ARENA_SIZEOF(fheap) >= (size + fheap->used))
 	      ? fheap : heaps);
 	 h; h = h->next) {
+	hp = h;
 	if (ARENA_SIZEOF(h) >= (n = size + h->used)) {
 	    void *ret;
 
@@ -566,7 +606,6 @@ zhalloc(size_t size)
 	}
     }
     {
-	Heap hp;
         /* not found, allocate new heap */
 #if defined(ZSH_MEM) && !defined(USE_MMAP)
 	static int called = 0;
@@ -575,7 +614,6 @@ zhalloc(size_t size)
 #endif
 
 	n = HEAP_ARENA_SIZE > size ? HEAPSIZE : size + sizeof(*h);
-	for (hp = NULL, h = heaps; h; hp = h, h = h->next);
 
 #ifdef USE_MMAP
 	h = mmap_heap_alloc(&n);
@@ -607,6 +645,7 @@ zhalloc(size_t size)
 	VALGRIND_MEMPOOL_ALLOC((char *)h, (char *)arena(h), req_size);
 #endif
 
+	DPUTS(hp && hp->next, "failed to find end of chain in zhalloc");
 	if (hp)
 	    hp->next = h;
 	else


  reply	other threads:[~2015-10-11 16:18 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-10 10:54 Sebastian Gniazdowski
2015-10-10 17:58 ` Bart Schaefer
2015-10-10 18:11   ` Sebastian Gniazdowski
2015-10-10 18:32     ` Sebastian Gniazdowski
2015-10-11  0:06       ` Bart Schaefer
2015-10-11  6:20         ` Bart Schaefer
2015-10-11  8:39           ` Sebastian Gniazdowski
2015-10-11 16:17             ` Bart Schaefer [this message]
2015-10-11 16:48               ` Sebastian Gniazdowski
2015-10-11 17:31                 ` Bart Schaefer
2015-10-11 18:05                   ` Sebastian Gniazdowski
2015-10-11 21:22                     ` Bart Schaefer
2015-10-12  8:21                       ` Sebastian Gniazdowski
2015-10-12 14:01                         ` Bart Schaefer
2015-10-12 16:50                           ` Sebastian Gniazdowski
2015-10-13  0:33                             ` Bart Schaefer
2015-10-13  8:21                               ` Sebastian Gniazdowski
2015-10-13 15:52                                 ` Bart Schaefer
2015-10-14  6:50                                   ` Sebastian Gniazdowski
2015-10-14 13:27                                   ` Peter Stephenson
2015-10-14 16:25                                     ` Bart Schaefer
2015-10-14 16:50                                       ` Bart Schaefer
2015-10-15  4:32                                         ` Bart Schaefer
2015-10-15 13:03                                           ` Sebastian Gniazdowski
2015-10-16  0:35                                             ` Bart Schaefer
2015-10-17  9:12                                               ` Sebastian Gniazdowski
2015-10-17  9:24                                                 ` Sebastian Gniazdowski
2015-10-18 16:19                                                 ` Bart Schaefer
2015-10-18 20:40                                                   ` Sebastian Gniazdowski
2015-10-18 21:07                                                     ` Bart Schaefer
2015-10-18 21:31                                                       ` Sebastian Gniazdowski
2015-10-19 17:21                                                         ` Bart Schaefer
2015-10-22 12:49                                                           ` Sebastian Gniazdowski
2015-10-22 15:00                                                             ` Bart Schaefer
2015-10-22 16:28                                                               ` Sebastian Gniazdowski
2015-10-22 16:33                                                                 ` Sebastian Gniazdowski
2015-10-23 15:40                                                               ` Sebastian Gniazdowski
2015-10-23 15:57                                                                 ` Sebastian Gniazdowski
2015-10-23 19:26                                                                   ` Bart Schaefer
2015-10-23 23:50                                                                     ` Bart Schaefer
2015-10-24  6:09                                                                       ` Sebastian Gniazdowski
2015-10-24  7:37                                                                         ` Sebastian Gniazdowski
2015-10-24  8:04                                                                           ` Sebastian Gniazdowski
2015-10-24 19:39                                                                           ` Bart Schaefer
2015-10-25  7:35                                                                             ` Sebastian Gniazdowski
2015-10-25 17:23                                                                               ` Bart Schaefer
2015-10-25 20:56                                                                                 ` Sebastian Gniazdowski
2015-10-26  0:49                                                                                   ` Bart Schaefer
2015-10-26  7:41                                                                                     ` Sebastian Gniazdowski
2015-10-26  7:47                                                                                       ` Sebastian Gniazdowski
2015-10-24  6:27                                                                       ` Sebastian Gniazdowski
2015-10-24 10:54                                                                       ` Sebastian Gniazdowski
2015-10-24 11:25                                                                         ` Sebastian Gniazdowski
2015-10-24 16:31                                                                         ` Bart Schaefer
2015-10-24 16:41                                                                           ` Bart Schaefer
2015-10-23  6:32                                                             ` Sebastian Gniazdowski
2015-10-16  0:37                                             ` Bart Schaefer
2015-10-13 13:07                         ` Sebastian Gniazdowski
2015-10-13 13:31                           ` Sebastian Gniazdowski
2015-10-12 12:05                       ` Sebastian Gniazdowski
2015-10-12 15:13                         ` Bart Schaefer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=151011091757.ZM27755@torch.brasslantern.com \
    --to=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).