From 6fbcc1bba971092be2a82097d34b5866f3511b01 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 15 Jan 2024 13:37:45 -0500 Subject: [PATCH 1/2] Cleanup: Use Map instead of GHash for memfile writing The goal is to simplify future work on #106903. Also include a couple other cleanups: removing "extern" for declarations in headers, and using the `LISTBASE_FOREACH` macro. --- source/blender/blenloader/BLO_undofile.hh | 14 +++---- source/blender/blenloader/intern/undofile.cc | 37 ++++--------------- source/blender/blenloader/intern/writefile.cc | 13 +++---- 3 files changed, 20 insertions(+), 44 deletions(-) diff --git a/source/blender/blenloader/BLO_undofile.hh b/source/blender/blenloader/BLO_undofile.hh index c15a2302d6c..8301bc7c579 100644 --- a/source/blender/blenloader/BLO_undofile.hh +++ b/source/blender/blenloader/BLO_undofile.hh @@ -11,8 +11,8 @@ #include "BLI_filereader.h" #include "BLI_listbase.h" +#include "BLI_map.hh" -struct GHash; struct Main; struct Scene; @@ -45,7 +45,7 @@ struct MemFileWriteData { MemFileChunk *reference_current_chunk; /** Maps an ID session uuid to its first reference MemFileChunk, if existing. */ - GHash *id_session_uuid_mapping; + blender::Map id_session_uuid_mapping; }; struct MemFileUndoData { @@ -80,25 +80,25 @@ void BLO_memfile_chunk_add(MemFileWriteData *mem_data, const char *buf, size_t s */ /* **************** support for memory-write, for undo buffers *************** */ -extern void BLO_memfile_free(MemFile *memfile); +void BLO_memfile_free(MemFile *memfile); /** * Result is that 'first' is being freed. * To keep the #MemFile linked list of consistent, `first` is always first in list. */ -extern void BLO_memfile_merge(MemFile *first, MemFile *second); +void BLO_memfile_merge(MemFile *first, MemFile *second); /** * Clear is_identical_future before adding next memfile. */ -extern void BLO_memfile_clear_future(MemFile *memfile); +void BLO_memfile_clear_future(MemFile *memfile); /* Utilities. */ -extern Main *BLO_memfile_main_get(MemFile *memfile, Main *bmain, Scene **r_scene); +Main *BLO_memfile_main_get(MemFile *memfile, Main *bmain, Scene **r_scene); /** * Saves .blend using undo buffer. * * \return success. */ -extern bool BLO_memfile_write_file(MemFile *memfile, const char *filepath); +bool BLO_memfile_write_file(MemFile *memfile, const char *filepath); FileReader *BLO_memfile_new_filereader(MemFile *memfile, int undo_direction); diff --git a/source/blender/blenloader/intern/undofile.cc b/source/blender/blenloader/intern/undofile.cc index 4cfc214ac90..c88f2dd6f68 100644 --- a/source/blender/blenloader/intern/undofile.cc +++ b/source/blender/blenloader/intern/undofile.cc @@ -25,7 +25,6 @@ #include "DNA_listBase.h" #include "BLI_blenlib.h" -#include "BLI_ghash.h" #include "BLO_readfile.h" #include "BLO_undofile.hh" @@ -54,27 +53,20 @@ void BLO_memfile_merge(MemFile *first, MemFile *second) { /* We use this mapping to store the memory buffers from second memfile chunks which are not owned * by it (i.e. shared with some previous memory steps). */ - GHash *buffer_to_second_memchunk = BLI_ghash_new( - BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__); + blender::Map buffer_to_second_memchunk; /* First, detect all memchunks in second memfile that are not owned by it. */ - for (MemFileChunk *sc = static_cast(second->chunks.first); sc != nullptr; - sc = static_cast(sc->next)) - { + LISTBASE_FOREACH (MemFileChunk *, sc, &first->chunks) { if (sc->is_identical) { - BLI_ghash_insert(buffer_to_second_memchunk, (void *)sc->buf, sc); + buffer_to_second_memchunk.add(sc->buf, sc); } } /* Now, check all chunks from first memfile (the one we are removing), and if a memchunk owned by * it is also used by the second memfile, transfer the ownership. */ - for (MemFileChunk *fc = static_cast(first->chunks.first); fc != nullptr; - fc = static_cast(fc->next)) - { + LISTBASE_FOREACH (MemFileChunk *, fc, &first->chunks) { if (!fc->is_identical) { - MemFileChunk *sc = static_cast( - BLI_ghash_lookup(buffer_to_second_memchunk, fc->buf)); - if (sc != nullptr) { + if (MemFileChunk *sc = buffer_to_second_memchunk.lookup_default(fc->buf, nullptr)) { BLI_assert(sc->is_identical); sc->is_identical = false; fc->is_identical = true; @@ -85,8 +77,6 @@ void BLO_memfile_merge(MemFile *first, MemFile *second) } } - BLI_ghash_free(buffer_to_second_memchunk, nullptr, nullptr); - BLO_memfile_free(first); } @@ -113,22 +103,11 @@ void BLO_memfile_write_init(MemFileWriteData *mem_data, * current Main data-base broke the order matching with the memchunks from previous step. */ if (reference_memfile != nullptr) { - mem_data->id_session_uuid_mapping = BLI_ghash_new( - BLI_ghashutil_inthash_p_simple, BLI_ghashutil_intcmp, __func__); uint current_session_uuid = MAIN_ID_SESSION_UUID_UNSET; LISTBASE_FOREACH (MemFileChunk *, mem_chunk, &reference_memfile->chunks) { if (!ELEM(mem_chunk->id_session_uuid, MAIN_ID_SESSION_UUID_UNSET, current_session_uuid)) { current_session_uuid = mem_chunk->id_session_uuid; - void **entry; - if (!BLI_ghash_ensure_p(mem_data->id_session_uuid_mapping, - POINTER_FROM_UINT(current_session_uuid), - &entry)) - { - *entry = mem_chunk; - } - else { - BLI_assert_unreachable(); - } + mem_data->id_session_uuid_mapping.add_new(current_session_uuid, mem_chunk); } } } @@ -136,9 +115,7 @@ void BLO_memfile_write_init(MemFileWriteData *mem_data, void BLO_memfile_write_finalize(MemFileWriteData *mem_data) { - if (mem_data->id_session_uuid_mapping != nullptr) { - BLI_ghash_free(mem_data->id_session_uuid_mapping, nullptr, nullptr); - } + mem_data->id_session_uuid_mapping.clear_and_shrink(); } void BLO_memfile_chunk_add(MemFileWriteData *mem_data, const char *buf, size_t size) diff --git a/source/blender/blenloader/intern/writefile.cc b/source/blender/blenloader/intern/writefile.cc index 4eeb342a036..00292239111 100644 --- a/source/blender/blenloader/intern/writefile.cc +++ b/source/blender/blenloader/intern/writefile.cc @@ -435,7 +435,7 @@ struct BlendWriter { static WriteData *writedata_new(WriteWrap *ww) { - WriteData *wd = static_cast(MEM_callocN(sizeof(*wd), "writedata")); + WriteData *wd = MEM_new(__func__); wd->sdna = DNA_sdna_current_get(); @@ -487,7 +487,7 @@ static void writedata_free(WriteData *wd) if (wd->buffer.buf) { MEM_freeN(wd->buffer.buf); } - MEM_freeN(wd); + MEM_delete(wd); } /** \} */ @@ -620,14 +620,13 @@ static void mywrite_id_begin(WriteData *wd, ID *id) MemFileChunk *prev_memchunk = curr_memchunk != nullptr ? static_cast(curr_memchunk->prev) : nullptr; - if (wd->mem.id_session_uuid_mapping != nullptr && - (curr_memchunk == nullptr || curr_memchunk->id_session_uuid != id->session_uuid || + if ((curr_memchunk == nullptr || curr_memchunk->id_session_uuid != id->session_uuid || (prev_memchunk != nullptr && (prev_memchunk->id_session_uuid == curr_memchunk->id_session_uuid)))) { - void *ref = BLI_ghash_lookup(wd->mem.id_session_uuid_mapping, - POINTER_FROM_UINT(id->session_uuid)); - if (ref != nullptr) { + if (MemFileChunk *ref = wd->mem.id_session_uuid_mapping.lookup_default(id->session_uuid, + nullptr)) + { wd->mem.reference_current_chunk = static_cast(ref); } /* Else, no existing memchunk found, i.e. this is supposed to be a new ID. */ -- 2.30.2 From 33bdaa400a63ed165cae61cecdee6f23afa8ac68 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 15 Jan 2024 13:56:53 -0500 Subject: [PATCH 2/2] Fix wrong list used for iteration --- source/blender/blenloader/intern/undofile.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/blender/blenloader/intern/undofile.cc b/source/blender/blenloader/intern/undofile.cc index c88f2dd6f68..245d713a3fe 100644 --- a/source/blender/blenloader/intern/undofile.cc +++ b/source/blender/blenloader/intern/undofile.cc @@ -56,7 +56,7 @@ void BLO_memfile_merge(MemFile *first, MemFile *second) blender::Map buffer_to_second_memchunk; /* First, detect all memchunks in second memfile that are not owned by it. */ - LISTBASE_FOREACH (MemFileChunk *, sc, &first->chunks) { + LISTBASE_FOREACH (MemFileChunk *, sc, &second->chunks) { if (sc->is_identical) { buffer_to_second_memchunk.add(sc->buf, sc); } -- 2.30.2