[PATCH 0/2] MR1133: Get rid of home grown allocator in DbgHelp.

[eric pouech (＠epo)]

Replace DbgHelp's internal memory allocator with heap functions:
- slight improvement in speed (~ 1 to 2 %)
- ability to reallocate chunks

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1133

[PATCH 1/2] dbghelp: Use heap functions for allocation.

[Eric Pouech]

From: Eric Pouech <eric.pouech(a)gmail.com>

Creating a dedicated heap for each module (as it was done for
the private home grown pools).

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>
---
 dlls/dbghelp/dbghelp_private.h |  6 +--
 dlls/dbghelp/storage.c         | 86 ++--------------------------------
 2 files changed, 5 insertions(+), 87 deletions(-)

diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h
index 5e63d949a43..ceb92aabc4f 100644
--- a/dlls/dbghelp/dbghelp_private.h
+++ b/dlls/dbghelp/dbghelp_private.h
@@ -34,13 +34,9 @@
 
 #include "cvconst.h"
 
-/* #define USE_STATS */
-
 struct pool /* poor's man */
 {
-    struct list arena_list;
-    struct list arena_full;
-    size_t      arena_size;
+    HANDLE      heap;
 };
 
 void     pool_init(struct pool* a, size_t arena_size) DECLSPEC_HIDDEN;
diff --git a/dlls/dbghelp/storage.c b/dlls/dbghelp/storage.c
index 48a43e6bc3d..735fb3bce7e 100644
--- a/dlls/dbghelp/storage.c
+++ b/dlls/dbghelp/storage.c
@@ -25,100 +25,22 @@
 #include "wine/debug.h"
 
 #include "dbghelp_private.h"
-#ifdef USE_STATS
-#include <math.h>
-#endif
 
 WINE_DEFAULT_DEBUG_CHANNEL(dbghelp);
 
-struct pool_arena
-{
-    struct list entry;
-    char       *current;
-    char       *end;
-};
-
-void pool_init(struct pool* a, size_t arena_size)
+void pool_init(struct pool* pool, size_t arena_size)
 {
-    list_init( &a->arena_list );
-    list_init( &a->arena_full );
-    a->arena_size = arena_size;
+    pool->heap = HeapCreate(HEAP_NO_SERIALIZE, arena_size, 0);
 }
 
 void pool_destroy(struct pool* pool)
 {
-    struct pool_arena*  arena;
-    struct pool_arena*  next;
-
-#ifdef USE_STATS
-    size_t alloc, used, num;
-
-    alloc = used = num = 0;
-    LIST_FOR_EACH_ENTRY( arena, &pool->arena_list, struct pool_arena, entry )
-    {
-        alloc += arena->end - (char *)arena;
-        used += arena->current - (char*)arena;
-        num++;
-    }
-    LIST_FOR_EACH_ENTRY( arena, &pool->arena_full, struct pool_arena, entry )
-    {
-        alloc += arena->end - (char *)arena;
-        used += arena->current - (char*)arena;
-        num++;
-    }
-    if (alloc == 0) alloc = 1;      /* avoid division by zero */
-    FIXME("STATS: pool %p has allocated %u kbytes, used %u kbytes in %u arenas, non-allocation ratio: %.2f%%\n",
-          pool, (unsigned)(alloc >> 10), (unsigned)(used >> 10), (unsigned)num,
-          100.0 - (float)used / (float)alloc * 100.0);
-#endif
-
-    LIST_FOR_EACH_ENTRY_SAFE( arena, next, &pool->arena_list, struct pool_arena, entry )
-    {
-        list_remove( &arena->entry );
-        HeapFree(GetProcessHeap(), 0, arena);
-    }
-    LIST_FOR_EACH_ENTRY_SAFE( arena, next, &pool->arena_full, struct pool_arena, entry )
-    {
-        list_remove( &arena->entry );
-        HeapFree(GetProcessHeap(), 0, arena);
-    }
+    HeapDestroy(pool->heap);
 }
 
 void* pool_alloc(struct pool* pool, size_t len)
 {
-    struct pool_arena*  arena;
-    void*               ret;
-    size_t size;
-
-    len = (len + 3) & ~3; /* round up size on DWORD boundary */
-
-    LIST_FOR_EACH_ENTRY( arena, &pool->arena_list, struct pool_arena, entry )
-    {
-        if (arena->end - arena->current >= len)
-        {
-            ret = arena->current;
-            arena->current += len;
-            if (arena->current + 16 >= arena->end)
-            {
-                list_remove( &arena->entry );
-                list_add_tail( &pool->arena_full, &arena->entry );
-            }
-            return ret;
-        }
-    }
-
-    size = max( pool->arena_size, len );
-    arena = HeapAlloc(GetProcessHeap(), 0, size + sizeof(struct pool_arena));
-    if (!arena) return NULL;
-
-    ret = arena + 1;
-    arena->current = (char*)ret + len;
-    arena->end = (char*)ret + size;
-    if (arena->current + 16 >= arena->end)
-        list_add_tail( &pool->arena_full, &arena->entry );
-    else
-        list_add_head( &pool->arena_list, &arena->entry );
-    return ret;
+    return HeapAlloc(pool->heap, 0, len);
 }
 
 char* pool_strdup(struct pool* pool, const char* str)
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/1133

[PATCH 2/2] dbghelp: Realloc array of buckets inside struct vector.

[Eric Pouech]

From: Eric Pouech <eric.pouech(a)gmail.com>

Instead of silently leaking no longer used chunks.
Be more robust to OOM conditions.
Introducing pool_realloc().

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>
---
 dlls/dbghelp/dbghelp_private.h |  1 +
 dlls/dbghelp/storage.c         | 28 +++++++++++++---------------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h
index ceb92aabc4f..daf15732877 100644
--- a/dlls/dbghelp/dbghelp_private.h
+++ b/dlls/dbghelp/dbghelp_private.h
@@ -42,6 +42,7 @@ struct pool /* poor's man */
 void     pool_init(struct pool* a, size_t arena_size) DECLSPEC_HIDDEN;
 void     pool_destroy(struct pool* a) DECLSPEC_HIDDEN;
 void*    pool_alloc(struct pool* a, size_t len) DECLSPEC_HIDDEN;
+void*    pool_realloc(struct pool* a, void* ptr, size_t len) DECLSPEC_HIDDEN;
 char*    pool_strdup(struct pool* a, const char* str) DECLSPEC_HIDDEN;
 
 struct vector
diff --git a/dlls/dbghelp/storage.c b/dlls/dbghelp/storage.c
index 735fb3bce7e..daed3aa8d99 100644
--- a/dlls/dbghelp/storage.c
+++ b/dlls/dbghelp/storage.c
@@ -43,6 +43,11 @@ void* pool_alloc(struct pool* pool, size_t len)
     return HeapAlloc(pool->heap, 0, len);
 }
 
+void* pool_realloc(struct pool* pool, void* ptr, size_t len)
+{
+    return ptr ? HeapReAlloc(pool->heap, 0, ptr, len) : pool_alloc(pool, len);
+}
+
 char* pool_strdup(struct pool* pool, const char* str)
 {
     char* ret;
@@ -90,32 +95,25 @@ void* vector_at(const struct vector* v, unsigned pos)
 
 void* vector_add(struct vector* v, struct pool* pool)
 {
-    unsigned    ncurr = v->num_elts++;
-
-    /* check that we don't wrap around */
-    assert(v->num_elts > ncurr);
-    if (ncurr == (v->num_buckets << v->shift))
+    if (v->num_elts == (v->num_buckets << v->shift))
     {
-        if(v->num_buckets == v->buckets_allocated)
+        if (v->num_buckets == v->buckets_allocated)
         {
             /* Double the bucket cache, so it scales well with big vectors.*/
-            unsigned    new_reserved;
+            unsigned    new_reserved = v->buckets_allocated ? v->buckets_allocated * 2 : 1;
             void*       new;
 
-            new_reserved = 2*v->buckets_allocated;
-            if(new_reserved == 0) new_reserved = 1;
-
-            /* Don't even try to resize memory.
-               Pool datastructure is very inefficient with reallocs. */
-            new = pool_alloc(pool, new_reserved * sizeof(void*));
-            memcpy(new, v->buckets, v->buckets_allocated * sizeof(void*));
+            new = pool_realloc(pool, v->buckets, new_reserved * sizeof(void*));
+            if (!new) return NULL;
             v->buckets = new;
             v->buckets_allocated = new_reserved;
         }
         v->buckets[v->num_buckets] = pool_alloc(pool, v->elt_size << v->shift);
+        if (!v->buckets[v->num_buckets]) return NULL;
+        v->num_elts++;
         return v->buckets[v->num_buckets++];
     }
-    return vector_at(v, ncurr);
+    return vector_at(v, v->num_elts++);
 }
 
 /* We construct the sparse array as two vectors (of equal size)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1133

[PATCH 2/2] dbghelp: Realloc array of buckets inside struct vector.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125235

Your paranoid android.


=== debian11 (build log) ===

Task: Could not create the win32 wineprefix: Failed to disable the crash dialogs: 
Task: WineTest did not produce the win32 report