From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9876 invoked by alias); 11 Oct 2015 16:18:09 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 36834 Received: (qmail 6945 invoked from network); 11 Oct 2015 16:18:06 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.0 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:message-id:date:in-reply-to:comments :references:to:subject:mime-version:content-type; bh=vXUCnTvioQnMEDQU9C2TVLHeaFcQka2QbafwQVxqEuk=; b=kQubfyo2VK7I4j7CV9QGPyyxIkZc03qUmLayB33ORIeh5enMQKGJZHq9PPuYYj6bZ9 +lVr8QHAkJjn67T5WpRMlOfHbkXjXkSaeFpKqlscRlXJLnG6FxraO+aiFvrBVEB4brLz mbYWqLJ5wzJZDe2XnaZndd9FkmhFIpalIcAurs5EChwfyDLzWKBikc11au1IDAHkciC+ dW22xuikTWbgDZfBekFilFTXeVrRpqQuhH/XcHEhfiYKIFvTQvixDSRQrPAUfdUYoZAE 4oPFhj4szVG/kOQlE/d5xz6nPkF0Rf6B3QghZesziPVC2o4zUBio2pzn5yX+nFW5TJUM 4jfQ== X-Gm-Message-State: ALoCoQm8ZG0xgZ/c5mWJjY0+nyNUlPAYQ7sHwJ9hOgJkNGuFGPQfnrayAL5v7uEXgRbk1skriAln X-Received: by 10.182.28.100 with SMTP id a4mr14160548obh.38.1444580281024; Sun, 11 Oct 2015 09:18:01 -0700 (PDT) From: Bart Schaefer Message-Id: <151011091757.ZM27755@torch.brasslantern.com> Date: Sun, 11 Oct 2015 09:17:57 -0700 In-Reply-To: Comments: In reply to Sebastian Gniazdowski "Re: Slowdown around 5.0.5-dev-0" (Oct 11, 10:39am) References: <151010105849.ZM10144@torch.brasslantern.com> <151010170623.ZM16166@torch.brasslantern.com> <151010232045.ZM12931@torch.brasslantern.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: Slowdown around 5.0.5-dev-0 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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