From 9f8e3e82dd2c2bb98f72b6e026b72d9c47d5ad62 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sun, 11 Oct 2015 21:11:10 -0700 Subject: 36834: freeheap preserves last allocated heap This is the first of two optimizations to improve heap performance when there are a large number of mostly-filled heap arenas. --- Src/mem.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 13 deletions(-) (limited to 'Src/mem.c') diff --git a/Src/mem.c b/Src/mem.c index b9569ea0c..d49f685fe 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 -- cgit v1.2.3 From d77bf2ba88c289e28139ce36ac767447113ab95d Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sun, 11 Oct 2015 21:16:58 -0700 Subject: 36836: zhalloc() avoids re-scanning all heaps when the last known heap with free space does not have enough space This is the second of two performance optimizations for situations where all heap arenas in the list are mostly full. --- ChangeLog | 5 ++++- Src/mem.c | 12 ++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) (limited to 'Src/mem.c') diff --git a/ChangeLog b/ChangeLog index 3adb1f159..421da2f6f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,9 @@ 2015-10-11 Barton E. Schaefer - * 36834: Src/mem.c: freeheap preserves last allocated heap + * 36836: Src/mem.c: zhalloc() avoids re-scanning all heaps when + the last known heap with free space does not have enough space + + * 36834: Src/mem.c: freeheap() preserves last allocated heap 2015-10-11 Frank Terbeck diff --git a/Src/mem.c b/Src/mem.c index d49f685fe..191428323 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -582,10 +582,14 @@ zhalloc(size_t size) /* find a heap with enough free space */ - for (h = ((fheap && ARENA_SIZEOF(fheap) >= (size + fheap->used)) - ? fheap : heaps); - h; h = h->next) { - hp = h; + /* + * This previously assigned: + * h = ((fheap && ARENA_SIZEOF(fheap) >= (size + fheap->used)) + * ? fheap : heaps); + * but we think that nothing upstream of fheap has more free space, + * so why start over at heaps just because fheap has too little? + */ + for (h = (fheap ? fheap : heaps); h; h = h->next) { if (ARENA_SIZEOF(h) >= (n = size + h->used)) { void *ret; -- cgit v1.2.3 From 643aca932aa30083246312eeddd2e0d6befa5861 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sun, 11 Oct 2015 21:48:10 -0700 Subject: One crucial assignment accidentally lost from 36834 when merging 36836. --- Src/mem.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'Src/mem.c') diff --git a/Src/mem.c b/Src/mem.c index 191428323..68bd76740 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -386,6 +386,8 @@ freeheap(void) VALGRIND_MEMPOOL_TRIM((char *)h, (char *)arena(h), h->used); #endif } else { + if (fheap == h) + fheap = NULL; if (h->next) { /* We want to cut this out of the arena list if we can */ if (h == heaps) @@ -404,8 +406,6 @@ freeheap(void) fheap = hl = h; break; } - if (fheap == h) - fheap = NULL; #ifdef USE_MMAP munmap((void *) h, h->size); #else @@ -590,6 +590,7 @@ zhalloc(size_t size) * so why start over at heaps just because fheap has too little? */ for (h = (fheap ? fheap : heaps); h; h = h->next) { + hp = h; if (ARENA_SIZEOF(h) >= (n = size + h->used)) { void *ret; -- cgit v1.2.3 From 506d5923802a930f24dd264588a8d33c05ed085b Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sat, 24 Oct 2015 13:43:21 -0700 Subject: 36943: restore scan for reclaimable blocks in freeheap() That scan had been removed by 36834, but testing showed memory usage climbing too high in cases where a new arena was always added at the end of the heap list. --- ChangeLog | 5 +++++ Src/mem.c | 10 ++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'Src/mem.c') diff --git a/ChangeLog b/ChangeLog index ef5cbe1f0..31fc78366 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-10-24 Barton E. Schaefer + + * 36943 (with updated comment): Src/mem.c: restore scan for + reclaimable blocks in freeheap() [had been removed by 36834] + 2015-10-24 Peter Stephenson * unposted: Src/utils.c: small typo. diff --git a/Src/mem.c b/Src/mem.c index 68bd76740..158ad9641 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -340,9 +340,15 @@ freeheap(void) * and performance goes to hell. * * Therefore, we defer freeing the most recently allocated arena until - * we reach popheap(). This may fail to reclaim some space in earlier - * arenas. + * we reach popheap(). * + * However, if the arena to which fheap points is unused, we want to + * reclaim space in earlier arenas, 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 -- cgit v1.2.3 From 8934007f25600150cee7e6eda2816c9110a9dc69 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Mon, 26 Oct 2015 07:19:01 -0700 Subject: 36956: revert 34451, mmap() is too slow on MacOS --- ChangeLog | 4 ++++ Src/mem.c | 7 +++++++ 2 files changed, 11 insertions(+) (limited to 'Src/mem.c') diff --git a/ChangeLog b/ChangeLog index b2291f5b1..a55439df4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2015-10-26 Barton E. Schaefer + + * 36956: Src/mem.c: revert 34451, mmap() is too slow on MacOS + 2015-10-26 Peter Stephenson * users/20825: Src/Zle/zle_utils.c: get_undo_current_change() diff --git a/Src/mem.c b/Src/mem.c index 158ad9641..3138bb1d5 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -79,9 +79,16 @@ #include +#if 0 +/* + * This change was designed to enable use of memory mapping on MacOS. + * However, performance tests indicate that MacOS mapped regions are + * significantly slower to allocate than memory from malloc(). + */ #if defined(MAP_ANON) && !defined(MAP_ANONYMOUS) #define MAP_ANONYMOUS MAP_ANON #endif +#endif /* 0 */ #if defined(MAP_ANONYMOUS) && defined(MAP_PRIVATE) -- cgit v1.2.3 From 81fa9fd25a57ebaf7e242bc678a6fd179bbdb718 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Mon, 26 Oct 2015 14:27:26 -0700 Subject: 36906: quite_signals() in ZSH_MEM realloc() --- ChangeLog | 3 +++ Src/mem.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'Src/mem.c') diff --git a/ChangeLog b/ChangeLog index df70cbd45..bdd27871d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2015-10-26 Barton E. Schaefer + * 36906: Kamil Dudka : Src/mem.c: + quite_signals() in ZSH_MEM realloc() + * 36968: Src/Modules/db_gdbm.c: use addmodulefd() to tell the shell about the descriptor of the dbm file diff --git a/Src/mem.c b/Src/mem.c index 3138bb1d5..62d18d0cd 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -1668,8 +1668,13 @@ realloc(MALLOC_RET_T p, MALLOC_ARG_T size) int i, l = 0; /* some system..., see above */ - if (!p && size) - return (MALLOC_RET_T) malloc(size); + if (!p && size) { + queue_signals(); + r = malloc(size); + unqueue_signals(); + return (MALLOC_RET_T) r; + } + /* and some systems even do this... */ if (!p || !size) return (MALLOC_RET_T) p; -- cgit v1.2.3 From 682e779a211fdfa0baa41c06d821f0ea9acf4941 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sat, 31 Oct 2015 09:54:04 -0700 Subject: unposted (cf. 36998,36999): undo 36956 / restore 34451 with expanded comment about the flip-flopping --- ChangeLog | 5 +++++ Completion/Unix/Command/_qemu | 4 ++-- Src/mem.c | 7 +++---- 3 files changed, 10 insertions(+), 6 deletions(-) (limited to 'Src/mem.c') diff --git a/ChangeLog b/ChangeLog index 499c871e4..a35b6056f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-10-31 Barton E. Schaefer + + * unposted (cf. 36998,36999): Src/mem.c: undo 36956 / restore 34451 + with expanded comment about the flip-flopping + 2015-10-31 Daniel Shahaf * 37032: Completion/Unix/Command/_git: Temporarily revert 36959. diff --git a/Completion/Unix/Command/_qemu b/Completion/Unix/Command/_qemu index db07eba33..3c21c3e74 100644 --- a/Completion/Unix/Command/_qemu +++ b/Completion/Unix/Command/_qemu @@ -2,7 +2,7 @@ _qemu_log_items () { local -a opts hline - $service -d \? | while read -A hline; do + $service -d \? 2>/dev/null | while read -A hline; do [[ $hline[1] = Log ]] && continue opts=($opts "${hline[1]}[${hline[2,-1]}]") done @@ -11,7 +11,7 @@ _qemu_log_items () { local _qemu_machines -_qemu_machines=(${${${(f)"$($service -M \?)"}[2,-1]}%% *}) +_qemu_machines=(${${${(f)"$($service -M \? 2>/dev/null)"}[2,-1]}%% *}) _arguments \ '-'{fda,fdb,hda,hdb,hdc,hdd,cdrom}':disk image:_files' \ diff --git a/Src/mem.c b/Src/mem.c index 62d18d0cd..e31145e1e 100644 --- a/Src/mem.c +++ b/Src/mem.c @@ -79,16 +79,15 @@ #include -#if 0 /* - * This change was designed to enable use of memory mapping on MacOS. + * This definition is designed to enable use of memory mapping on MacOS. * However, performance tests indicate that MacOS mapped regions are - * significantly slower to allocate than memory from malloc(). + * somewhat slower to allocate than memory from malloc(), so whether + * using this improves performance depends on details of zhalloc(). */ #if defined(MAP_ANON) && !defined(MAP_ANONYMOUS) #define MAP_ANONYMOUS MAP_ANON #endif -#endif /* 0 */ #if defined(MAP_ANONYMOUS) && defined(MAP_PRIVATE) -- cgit v1.2.3