From 7d44065f7368e62cceaa9469b12657adb69e37ef Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 27 Dec 2023 12:21:39 -0500 Subject: [PATCH] Cleanup: Revert replacement of GSQueue with std::queue There are some tragic design flaws with the Microsoft STL implementation of `std::dequeue`. Unless we implement our own similar data structure or use an implementation from another library, the change isn't worth it. This reverts commit b26cd6a4b9008836f9d764489b4aac5e143ad611. This reverts commit cc11ba33d90e5a541e8263f7d365d6eeda9d984c. This reverts commit c929d750541bc25131dec63c9c9e126a1370daf0. This reverts commit bd3d5a750d2d0f5f72ed7ae262cd69abec4b0143. --- intern/ghost/test/CMakeLists.txt | 1 + .../blenkernel/intern/multires_unsubdivide.cc | 61 ++++--- source/blender/blenlib/BLI_gsqueue.h | 48 +++++ source/blender/blenlib/CMakeLists.txt | 2 + source/blender/blenlib/intern/gsqueue.c | 164 ++++++++++++++++++ source/blender/blenlib/intern/threads.cc | 40 +++-- .../blender/depsgraph/intern/eval/deg_eval.cc | 18 +- source/blender/makesdna/intern/CMakeLists.txt | 1 + 8 files changed, 282 insertions(+), 53 deletions(-) create mode 100644 source/blender/blenlib/BLI_gsqueue.h create mode 100644 source/blender/blenlib/intern/gsqueue.c diff --git a/intern/ghost/test/CMakeLists.txt b/intern/ghost/test/CMakeLists.txt index 2bd5ee48661..e6896fbbaf4 100644 --- a/intern/ghost/test/CMakeLists.txt +++ b/intern/ghost/test/CMakeLists.txt @@ -165,6 +165,7 @@ add_definitions(-DMATH_STANDALONE) add_library(bli_lib "../../../source/blender/blenlib/intern/fileops.c" + "../../../source/blender/blenlib/intern/gsqueue.c" "../../../source/blender/blenlib/intern/rct.c" "../../../source/blender/blenlib/intern/string.c" "../../../source/blender/blenlib/intern/string_utf8.c" diff --git a/source/blender/blenkernel/intern/multires_unsubdivide.cc b/source/blender/blenkernel/intern/multires_unsubdivide.cc index 3f3bafce9e7..f9a7a349354 100644 --- a/source/blender/blenkernel/intern/multires_unsubdivide.cc +++ b/source/blender/blenkernel/intern/multires_unsubdivide.cc @@ -9,8 +9,6 @@ * its corresponding grids to match a given original mesh. */ -#include - #include "MEM_guardedalloc.h" #include "DNA_mesh_types.h" @@ -18,6 +16,7 @@ #include "DNA_modifier_types.h" #include "DNA_scene_types.h" +#include "BLI_gsqueue.h" #include "BLI_math_vector.h" #include "BKE_customdata.hh" @@ -165,7 +164,8 @@ static bool is_vertex_diagonal(BMVert *from_v, BMVert *to_v) static void unsubdivide_face_center_vertex_tag(BMesh *bm, BMVert *initial_vertex) { blender::BitVector<> visited_verts(bm->totvert); - std::queue queue; + GSQueue *queue; + queue = BLI_gsqueue_new(sizeof(BMVert *)); /* Add and tag the vertices connected by a diagonal to initial_vertex to the flood fill queue. If * initial_vertex is a pole and there is a valid solution, those vertices should be the (0,0) of @@ -178,7 +178,7 @@ static void unsubdivide_face_center_vertex_tag(BMesh *bm, BMVert *initial_vertex BM_ITER_ELEM (neighbor_v, &iter_a, f, BM_VERTS_OF_FACE) { int neighbor_vertex_index = BM_elem_index_get(neighbor_v); if (neighbor_v != initial_vertex && is_vertex_diagonal(neighbor_v, initial_vertex)) { - queue.push(neighbor_v); + BLI_gsqueue_push(queue, &neighbor_v); visited_verts[neighbor_vertex_index].set(); BM_elem_flag_set(neighbor_v, BM_ELEM_TAG, true); } @@ -190,39 +190,43 @@ static void unsubdivide_face_center_vertex_tag(BMesh *bm, BMVert *initial_vertex * direction. If a solution exists and `initial_vertex` was a pole, this is guaranteed that will * tag all the (0,0) vertices of the grids, and nothing else. */ /* If it was not a pole, it may or may not find a solution, even if the solution exists. */ - while (!queue.empty()) { - BMVert *from_v = queue.front(); - queue.pop(); + while (!BLI_gsqueue_is_empty(queue)) { + BMVert *from_v; + BLI_gsqueue_pop(queue, &from_v); /* Get the diagonals (first connected step) */ - std::queue diagonals; + GSQueue *diagonals; + diagonals = BLI_gsqueue_new(sizeof(BMVert *)); BM_ITER_ELEM (f, &iter, from_v, BM_FACES_OF_VERT) { BM_ITER_ELEM (neighbor_v, &iter_a, f, BM_VERTS_OF_FACE) { if (neighbor_v != from_v && is_vertex_diagonal(neighbor_v, from_v)) { - diagonals.push(neighbor_v); + BLI_gsqueue_push(diagonals, &neighbor_v); } } } /* Do the second connected step. This vertices are the ones that are added to the flood fill * queue. */ - while (!diagonals.empty()) { - BMVert *diagonal_v = diagonals.front(); - diagonals.pop(); + while (!BLI_gsqueue_is_empty(diagonals)) { + BMVert *diagonal_v; + BLI_gsqueue_pop(diagonals, &diagonal_v); BM_ITER_ELEM (f, &iter, diagonal_v, BM_FACES_OF_VERT) { BM_ITER_ELEM (neighbor_v, &iter_a, f, BM_VERTS_OF_FACE) { int neighbor_vertex_index = BM_elem_index_get(neighbor_v); if (!visited_verts[neighbor_vertex_index] && neighbor_v != diagonal_v && is_vertex_diagonal(neighbor_v, diagonal_v)) { - queue.push(neighbor_v); - visited_verts[neighbor_vertex_index].set(); + BLI_gsqueue_push(queue, &neighbor_v); + visited_verts[neighbor_vertex_index].set(true); BM_elem_flag_set(neighbor_v, BM_ELEM_TAG, true); } } } } + BLI_gsqueue_free(diagonals); } + + BLI_gsqueue_free(queue); } /** @@ -286,10 +290,10 @@ static bool unsubdivide_tag_disconnected_mesh_element(BMesh *bm, int *elem_id, i /* First, get vertex candidates to try to generate possible un-subdivide solution. */ /* Find a vertex pole. If there is a solution on an all quad base mesh, this vertex should be * part of the base mesh. If it isn't, then there is no solution. */ - std::queue initial_vertex; + GSQueue *initial_vertex = BLI_gsqueue_new(sizeof(BMVert *)); BMVert *initial_vertex_pole = unsubdivide_find_any_pole(bm, elem_id, elem); if (initial_vertex_pole != nullptr) { - initial_vertex.push(initial_vertex_pole); + BLI_gsqueue_push(initial_vertex, &initial_vertex_pole); } /* Also try from the different 4 vertices of a quad in the current @@ -311,16 +315,16 @@ static bool unsubdivide_tag_disconnected_mesh_element(BMesh *bm, int *elem_id, i } BM_ITER_ELEM (v, &iter_a, init_face, BM_VERTS_OF_FACE) { - initial_vertex.push(v); + BLI_gsqueue_push(initial_vertex, &v); } bool valid_tag_found = false; /* Check all vertex candidates to a solution. */ - while (!initial_vertex.empty()) { + while (!BLI_gsqueue_is_empty(initial_vertex)) { - BMVert *iv = initial_vertex.front(); - initial_vertex.pop(); + BMVert *iv; + BLI_gsqueue_pop(initial_vertex, &iv); /* Generate a possible solution. */ unsubdivide_face_center_vertex_tag(bm, iv); @@ -341,6 +345,7 @@ static bool unsubdivide_tag_disconnected_mesh_element(BMesh *bm, int *elem_id, i } } } + BLI_gsqueue_free(initial_vertex); return valid_tag_found; } @@ -354,29 +359,31 @@ static int unsubdivide_init_elem_ids(BMesh *bm, int *elem_id) int current_id = 0; for (int i = 0; i < bm->totvert; i++) { if (!visited_verts[i]) { - std::queue queue; + GSQueue *queue; + queue = BLI_gsqueue_new(sizeof(BMVert *)); visited_verts[i] = true; elem_id[i] = current_id; - queue.push(BM_vert_at_index(bm, i)); + BMVert *iv = BM_vert_at_index(bm, i); + BLI_gsqueue_push(queue, &iv); - while (!queue.empty()) { + while (!BLI_gsqueue_is_empty(queue)) { BMIter iter; - BMVert *current_v = queue.front(); - queue.pop(); - BMVert *neighbor_v; + BMVert *current_v, *neighbor_v; BMEdge *ed; + BLI_gsqueue_pop(queue, ¤t_v); BM_ITER_ELEM (ed, &iter, current_v, BM_EDGES_OF_VERT) { neighbor_v = BM_edge_other_vert(ed, current_v); const int neighbor_index = BM_elem_index_get(neighbor_v); if (!visited_verts[neighbor_index]) { visited_verts[neighbor_index] = true; elem_id[neighbor_index] = current_id; - queue.push(neighbor_v); + BLI_gsqueue_push(queue, &neighbor_v); } } } current_id++; + BLI_gsqueue_free(queue); } } MEM_freeN(visited_verts); diff --git a/source/blender/blenlib/BLI_gsqueue.h b/source/blender/blenlib/BLI_gsqueue.h new file mode 100644 index 00000000000..02c0527a0a6 --- /dev/null +++ b/source/blender/blenlib/BLI_gsqueue.h @@ -0,0 +1,48 @@ +/* SPDX-FileCopyrightText: 2001-2002 NaN Holding BV. All rights reserved. + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +#pragma once + +/** \file + * \ingroup bli + */ + +#include "BLI_utildefines.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct _GSQueue GSQueue; + +GSQueue *BLI_gsqueue_new(size_t elem_size); +/** + * Returns true if the queue is empty, false otherwise. + */ +bool BLI_gsqueue_is_empty(const GSQueue *queue); +size_t BLI_gsqueue_len(const GSQueue *queue); +/** + * Retrieves and removes the first element from the queue. + * The value is copies to \a r_item, which must be at least \a elem_size bytes. + * + * Does not reduce amount of allocated memory. + */ +void BLI_gsqueue_pop(GSQueue *queue, void *r_item); +/** + * Copies the source value onto the end of the queue + * + * \note This copies #GSQueue.elem_size bytes from \a item, + * (the pointer itself is not stored). + * + * \param item: source data to be copied to the queue. + */ +void BLI_gsqueue_push(GSQueue *queue, const void *item); +/** + * Free the queue's data and the queue itself. + */ +void BLI_gsqueue_free(GSQueue *queue); + +#ifdef __cplusplus +} +#endif diff --git a/source/blender/blenlib/CMakeLists.txt b/source/blender/blenlib/CMakeLists.txt index faef5fb9d09..3de5abb69e9 100644 --- a/source/blender/blenlib/CMakeLists.txt +++ b/source/blender/blenlib/CMakeLists.txt @@ -76,6 +76,7 @@ set(SRC intern/generic_vector_array.cc intern/generic_virtual_array.cc intern/generic_virtual_vector_array.cc + intern/gsqueue.c intern/hash_md5.c intern/hash_mm2a.c intern/hash_mm3.c @@ -238,6 +239,7 @@ set(SRC BLI_generic_virtual_array.hh BLI_generic_virtual_vector_array.hh BLI_ghash.h + BLI_gsqueue.h BLI_hash.h BLI_hash.hh BLI_hash_md5.h diff --git a/source/blender/blenlib/intern/gsqueue.c b/source/blender/blenlib/intern/gsqueue.c new file mode 100644 index 00000000000..55cfbd65655 --- /dev/null +++ b/source/blender/blenlib/intern/gsqueue.c @@ -0,0 +1,164 @@ +/* SPDX-FileCopyrightText: 2023 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup bli + * + * \brief A generic structure queue + * (a queue for fixed length generally small) structures. + */ + +#include + +#include "MEM_guardedalloc.h" + +#include "BLI_gsqueue.h" +#include "BLI_strict_flags.h" +#include "BLI_utildefines.h" + +/* target chunk size: 64kb */ +#define CHUNK_SIZE_DEFAULT (1 << 16) +/* ensure we get at least this many elems per chunk */ +#define CHUNK_ELEM_MIN 32 + +struct QueueChunk { + struct QueueChunk *next; + char data[0]; +}; + +struct _GSQueue { + struct QueueChunk *chunk_first; /* first active chunk to pop from */ + struct QueueChunk *chunk_last; /* last active chunk to push onto */ + struct QueueChunk *chunk_free; /* free chunks to reuse */ + size_t chunk_first_index; /* index into 'chunk_first' */ + size_t chunk_last_index; /* index into 'chunk_last' */ + size_t chunk_elem_max; /* number of elements per chunk */ + size_t elem_size; /* memory size of elements */ + size_t elem_num; /* total number of elements */ +}; + +static void *queue_get_first_elem(GSQueue *queue) +{ + return ((char *)(queue)->chunk_first->data) + ((queue)->elem_size * (queue)->chunk_first_index); +} + +static void *queue_get_last_elem(GSQueue *queue) +{ + return ((char *)(queue)->chunk_last->data) + ((queue)->elem_size * (queue)->chunk_last_index); +} + +/** + * \return number of elements per chunk, optimized for slop-space. + */ +static size_t queue_chunk_elem_max_calc(const size_t elem_size, size_t chunk_size) +{ + /* get at least this number of elems per chunk */ + const size_t elem_size_min = elem_size * CHUNK_ELEM_MIN; + + BLI_assert((elem_size != 0) && (chunk_size != 0)); + + while (UNLIKELY(chunk_size <= elem_size_min)) { + chunk_size <<= 1; + } + + /* account for slop-space */ + chunk_size -= (sizeof(struct QueueChunk) + MEM_SIZE_OVERHEAD); + + return chunk_size / elem_size; +} + +GSQueue *BLI_gsqueue_new(const size_t elem_size) +{ + GSQueue *queue = MEM_callocN(sizeof(*queue), "BLI_gsqueue_new"); + + queue->chunk_elem_max = queue_chunk_elem_max_calc(elem_size, CHUNK_SIZE_DEFAULT); + queue->elem_size = elem_size; + /* force init */ + queue->chunk_last_index = queue->chunk_elem_max - 1; + + return queue; +} + +static void queue_free_chunk(struct QueueChunk *data) +{ + while (data) { + struct QueueChunk *data_next = data->next; + MEM_freeN(data); + data = data_next; + } +} + +void BLI_gsqueue_free(GSQueue *queue) +{ + queue_free_chunk(queue->chunk_first); + queue_free_chunk(queue->chunk_free); + MEM_freeN(queue); +} + +void BLI_gsqueue_push(GSQueue *queue, const void *item) +{ + queue->chunk_last_index++; + queue->elem_num++; + + if (UNLIKELY(queue->chunk_last_index == queue->chunk_elem_max)) { + struct QueueChunk *chunk; + if (queue->chunk_free) { + chunk = queue->chunk_free; + queue->chunk_free = chunk->next; + } + else { + chunk = MEM_mallocN(sizeof(*chunk) + (queue->elem_size * queue->chunk_elem_max), __func__); + } + + chunk->next = NULL; + + if (queue->chunk_last == NULL) { + queue->chunk_first = chunk; + } + else { + queue->chunk_last->next = chunk; + } + + queue->chunk_last = chunk; + queue->chunk_last_index = 0; + } + + BLI_assert(queue->chunk_last_index < queue->chunk_elem_max); + + /* Return last of queue */ + memcpy(queue_get_last_elem(queue), item, queue->elem_size); +} + +void BLI_gsqueue_pop(GSQueue *queue, void *r_item) +{ + BLI_assert(BLI_gsqueue_is_empty(queue) == false); + + memcpy(r_item, queue_get_first_elem(queue), queue->elem_size); + queue->chunk_first_index++; + queue->elem_num--; + + if (UNLIKELY(queue->chunk_first_index == queue->chunk_elem_max || queue->elem_num == 0)) { + struct QueueChunk *chunk_free = queue->chunk_first; + + queue->chunk_first = queue->chunk_first->next; + queue->chunk_first_index = 0; + if (queue->chunk_first == NULL) { + queue->chunk_last = NULL; + queue->chunk_last_index = queue->chunk_elem_max - 1; + } + + chunk_free->next = queue->chunk_free; + queue->chunk_free = chunk_free; + } +} + +size_t BLI_gsqueue_len(const GSQueue *queue) +{ + return queue->elem_num; +} + +bool BLI_gsqueue_is_empty(const GSQueue *queue) +{ + return (queue->chunk_first == NULL); +} diff --git a/source/blender/blenlib/intern/threads.cc b/source/blender/blenlib/intern/threads.cc index 90284d3cb2d..fc9e3bae149 100644 --- a/source/blender/blenlib/intern/threads.cc +++ b/source/blender/blenlib/intern/threads.cc @@ -9,10 +9,10 @@ #include #include #include -#include #include "MEM_guardedalloc.h" +#include "BLI_gsqueue.h" #include "BLI_listbase.h" #include "BLI_system.h" #include "BLI_task.h" @@ -578,7 +578,7 @@ void BLI_condition_end(ThreadCondition *cond) /* ************************************************ */ struct ThreadQueue { - std::queue queue; + GSQueue *queue; pthread_mutex_t mutex; pthread_cond_t push_cond; pthread_cond_t finish_cond; @@ -588,7 +588,11 @@ struct ThreadQueue { ThreadQueue *BLI_thread_queue_init() { - ThreadQueue *queue = MEM_new(__func__); + ThreadQueue *queue; + + queue = static_cast(MEM_callocN(sizeof(ThreadQueue), "ThreadQueue")); + queue->queue = BLI_gsqueue_new(sizeof(void *)); + pthread_mutex_init(&queue->mutex, nullptr); pthread_cond_init(&queue->push_cond, nullptr); pthread_cond_init(&queue->finish_cond, nullptr); @@ -603,14 +607,16 @@ void BLI_thread_queue_free(ThreadQueue *queue) pthread_cond_destroy(&queue->push_cond); pthread_mutex_destroy(&queue->mutex); - MEM_delete(queue); + BLI_gsqueue_free(queue->queue); + + MEM_freeN(queue); } void BLI_thread_queue_push(ThreadQueue *queue, void *work) { pthread_mutex_lock(&queue->mutex); - queue->queue.push(work); + BLI_gsqueue_push(queue->queue, &work); /* signal threads waiting to pop */ pthread_cond_signal(&queue->push_cond); @@ -623,16 +629,15 @@ void *BLI_thread_queue_pop(ThreadQueue *queue) /* wait until there is work */ pthread_mutex_lock(&queue->mutex); - while (queue->queue.empty() && !queue->nowait) { + while (BLI_gsqueue_is_empty(queue->queue) && !queue->nowait) { pthread_cond_wait(&queue->push_cond, &queue->mutex); } /* if we have something, pop it */ - if (!queue->queue.empty()) { - work = queue->queue.front(); - queue->queue.pop(); + if (!BLI_gsqueue_is_empty(queue->queue)) { + BLI_gsqueue_pop(queue->queue, &work); - if (queue->queue.empty()) { + if (BLI_gsqueue_is_empty(queue->queue)) { pthread_cond_broadcast(&queue->finish_cond); } } @@ -688,7 +693,7 @@ void *BLI_thread_queue_pop_timeout(ThreadQueue *queue, int ms) /* wait until there is work */ pthread_mutex_lock(&queue->mutex); - while (queue->queue.empty() && !queue->nowait) { + while (BLI_gsqueue_is_empty(queue->queue) && !queue->nowait) { if (pthread_cond_timedwait(&queue->push_cond, &queue->mutex, &timeout) == ETIMEDOUT) { break; } @@ -698,11 +703,10 @@ void *BLI_thread_queue_pop_timeout(ThreadQueue *queue, int ms) } /* if we have something, pop it */ - if (!queue->queue.empty()) { - work = queue->queue.front(); - queue->queue.pop(); + if (!BLI_gsqueue_is_empty(queue->queue)) { + BLI_gsqueue_pop(queue->queue, &work); - if (queue->queue.empty()) { + if (BLI_gsqueue_is_empty(queue->queue)) { pthread_cond_broadcast(&queue->finish_cond); } } @@ -717,7 +721,7 @@ int BLI_thread_queue_len(ThreadQueue *queue) int size; pthread_mutex_lock(&queue->mutex); - size = queue->queue.size(); + size = BLI_gsqueue_len(queue->queue); pthread_mutex_unlock(&queue->mutex); return size; @@ -728,7 +732,7 @@ bool BLI_thread_queue_is_empty(ThreadQueue *queue) bool is_empty; pthread_mutex_lock(&queue->mutex); - is_empty = queue->queue.empty(); + is_empty = BLI_gsqueue_is_empty(queue->queue); pthread_mutex_unlock(&queue->mutex); return is_empty; @@ -750,7 +754,7 @@ void BLI_thread_queue_wait_finish(ThreadQueue *queue) /* wait for finish condition */ pthread_mutex_lock(&queue->mutex); - while (!queue->queue.empty()) { + while (!BLI_gsqueue_is_empty(queue->queue)) { pthread_cond_wait(&queue->finish_cond, &queue->mutex); } diff --git a/source/blender/depsgraph/intern/eval/deg_eval.cc b/source/blender/depsgraph/intern/eval/deg_eval.cc index 4b208dd6125..552602c9837 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval.cc @@ -8,14 +8,13 @@ * Evaluation engine entry-points for Depsgraph Engine. */ -#include - #include "intern/eval/deg_eval.h" #include "PIL_time.h" #include "BLI_compiler_attrs.h" #include "BLI_function_ref.hh" +#include "BLI_gsqueue.h" #include "BLI_task.h" #include "BLI_utildefines.h" @@ -336,18 +335,21 @@ void evaluate_graph_single_threaded_if_needed(DepsgraphEvalState *state) state->stage = EvaluationStage::SINGLE_THREADED_WORKAROUND; - std::queue evaluation_queue; - auto schedule_node_to_queue = [&](OperationNode *node) { evaluation_queue.push(node); }; - + GSQueue *evaluation_queue = BLI_gsqueue_new(sizeof(OperationNode *)); + auto schedule_node_to_queue = [&](OperationNode *node) { + BLI_gsqueue_push(evaluation_queue, &node); + }; schedule_graph(state, schedule_node_to_queue); - while (!evaluation_queue.empty()) { - OperationNode *operation_node = evaluation_queue.front(); - evaluation_queue.pop(); + while (!BLI_gsqueue_is_empty(evaluation_queue)) { + OperationNode *operation_node; + BLI_gsqueue_pop(evaluation_queue, &operation_node); evaluate_node(state, operation_node); schedule_children(state, operation_node, schedule_node_to_queue); } + + BLI_gsqueue_free(evaluation_queue); } void depsgraph_ensure_view_layer(Depsgraph *graph) diff --git a/source/blender/makesdna/intern/CMakeLists.txt b/source/blender/makesdna/intern/CMakeLists.txt index dabdfbe9e3d..7fe53d2f044 100644 --- a/source/blender/makesdna/intern/CMakeLists.txt +++ b/source/blender/makesdna/intern/CMakeLists.txt @@ -60,6 +60,7 @@ set(SRC_BLENLIB ../../blenlib/intern/hash_mm2a.c # Dependencies of BLI_mempool.c when ASAN is enabled. + ../../blenlib/intern/gsqueue.c ../../blenlib/intern/threads.cc ../../blenlib/intern/time.c )