Commit: 948211679f2a0681421160be0d3b90f507bc0be7
Author: Jeroen Bakker
Date:   Tue Jan 25 14:51:35 2022 +0100
Branches: master
https://developer.blender.org/rB948211679f2a0681421160be0d3b90f507bc0be7

Performance: Remap multiple items in UI

During sprite fright loading of complex scenes would spend a long time in 
remapping ID's
The remapping process is done on a per ID instance that resulted in a very time 
consuming
process that goes over every possible ID reference to find out if it needs to 
be updated.

If there are N of references to ID blocks and there are M ID blocks that needed 
to be remapped
it would take N*M checks. These checks are scattered around the place and 
memory.
Each reference would only be updated at most once, but most of the time no 
update is needed at all.

Idea: By grouping the changes together will reduce the number of checks 
resulting in improved performance.
This would only require N checks. Additional benefits is improved data locality 
as data is only loaded once
in the L2 cache.

It has be implemented for the resyncing process and UI editors.
On an Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz 16Gig the resyncing process went
from 170 seconds to 145 seconds (during hotspot recording).

After this patch has been applied we could add similar approach
to references (references between data blocks) and functionality (tagged 
deletion).
In my understanding this could reduce the resyncing process to less than a 
second.
Opening the village production file between 10 and 20 seconds.

Flame graphs showing that UI remapping isn't visible anymore 
(`WM_main_remap_editor_id_reference`)
* Master {F12769210 size=full}
* This patch {F12769211 size=full}

Reviewed By: mont29

Maniphest Tasks: T94185

Differential Revision: https://developer.blender.org/D13615

===================================================================

M       source/blender/blenkernel/BKE_lib_remap.h
M       source/blender/blenkernel/BKE_screen.h
M       source/blender/blenkernel/CMakeLists.txt
M       source/blender/blenkernel/intern/lib_id_delete.c
A       source/blender/blenkernel/intern/lib_id_remapper.cc
A       source/blender/blenkernel/intern/lib_id_remapper_test.cc
M       source/blender/blenkernel/intern/lib_override.c
M       source/blender/blenkernel/intern/lib_remap.c
M       source/blender/editors/include/ED_util.h
M       source/blender/editors/space_action/space_action.c
M       source/blender/editors/space_buttons/space_buttons.c
M       source/blender/editors/space_clip/space_clip.c
M       source/blender/editors/space_file/space_file.c
M       source/blender/editors/space_graph/space_graph.c
M       source/blender/editors/space_image/space_image.c
M       source/blender/editors/space_nla/space_nla.c
M       source/blender/editors/space_node/space_node.cc
M       source/blender/editors/space_outliner/space_outliner.cc
M       source/blender/editors/space_sequencer/space_sequencer.c
M       source/blender/editors/space_spreadsheet/space_spreadsheet.cc
M       source/blender/editors/space_text/space_text.c
M       source/blender/editors/space_view3d/space_view3d.c
M       source/blender/editors/util/ed_util.c
M       source/blender/windowmanager/WM_api.h
M       source/blender/windowmanager/intern/wm_event_system.c
M       source/blender/windowmanager/intern/wm_init_exit.c

===================================================================

diff --git a/source/blender/blenkernel/BKE_lib_remap.h 
b/source/blender/blenkernel/BKE_lib_remap.h
index d8842dbce7f..cc970342fbb 100644
--- a/source/blender/blenkernel/BKE_lib_remap.h
+++ b/source/blender/blenkernel/BKE_lib_remap.h
@@ -38,6 +38,9 @@
 extern "C" {
 #endif
 
+struct ID;
+struct IDRemapper;
+
 /* BKE_libblock_free, delete are declared in BKE_lib_id.h for convenience. */
 
 /* Also IDRemap->flag. */
@@ -97,6 +100,19 @@ enum {
   ID_REMAP_FORCE_OBDATA_IN_EDITMODE = 1 << 9,
 };
 
+/**
+ * Replace all references in given Main using the given \a mappings
+ *
+ * \note Is preferred over BKE_libblock_remap_locked due to performance.
+ */
+void BKE_libblock_remap_multiple_locked(struct Main *bmain,
+                                        const struct IDRemapper *mappings,
+                                        const short remap_flags);
+
+void BKE_libblock_remap_multiple(struct Main *bmain,
+                                 const struct IDRemapper *mappings,
+                                 const short remap_flags);
+
 /**
  * Replace all references in given Main to \a old_id by \a new_id
  * (if \a new_id is NULL, it unlinks \a old_id).
@@ -146,12 +162,61 @@ void BKE_libblock_relink_to_newid(struct Main *bmain, 
struct ID *id, int remap_f
     ATTR_NONNULL();
 
 typedef void (*BKE_library_free_notifier_reference_cb)(const void *);
-typedef void (*BKE_library_remap_editor_id_reference_cb)(struct ID *, struct 
ID *);
+typedef void (*BKE_library_remap_editor_id_reference_cb)(const struct 
IDRemapper *mappings);
 
 void 
BKE_library_callback_free_notifier_reference_set(BKE_library_free_notifier_reference_cb
 func);
 void BKE_library_callback_remap_editor_id_reference_set(
     BKE_library_remap_editor_id_reference_cb func);
 
+/* IDRemapper */
+struct IDRemapper;
+typedef enum IDRemapperApplyResult {
+  /** No remapping rules available for the source. */
+  ID_REMAP_RESULT_SOURCE_UNAVAILABLE,
+  /** Source isn't mappable (e.g. NULL). */
+  ID_REMAP_RESULT_SOURCE_NOT_MAPPABLE,
+  /** Source has been remapped to a new pointer. */
+  ID_REMAP_RESULT_SOURCE_REMAPPED,
+  /** Source has been set to NULL. */
+  ID_REMAP_RESULT_SOURCE_UNASSIGNED,
+} IDRemapperApplyResult;
+
+typedef enum IDRemapperApplyOptions {
+  ID_REMAP_APPLY_UPDATE_REFCOUNT = (1 << 0),
+  ID_REMAP_APPLY_ENSURE_REAL = (1 << 1),
+
+  ID_REMAP_APPLY_DEFAULT = 0,
+} IDRemapperApplyOptions;
+
+typedef void (*IDRemapperIterFunction)(struct ID *old_id, struct ID *new_id, 
void *user_data);
+
+/**
+ * Create a new ID Remapper.
+ *
+ * An ID remapper stores multiple remapping rules.
+ */
+struct IDRemapper *BKE_id_remapper_create(void);
+
+void BKE_id_remapper_clear(struct IDRemapper *id_remapper);
+bool BKE_id_remapper_is_empty(const struct IDRemapper *id_remapper);
+/** Free the given ID Remapper. */
+void BKE_id_remapper_free(struct IDRemapper *id_remapper);
+/** Add a new remapping. */
+void BKE_id_remapper_add(struct IDRemapper *id_remapper, struct ID *old_id, 
struct ID *new_id);
+
+/**
+ * Apply a remapping.
+ *
+ * Update the id pointer stored in the given r_id_ptr if a remapping rule 
exists.
+ */
+IDRemapperApplyResult BKE_id_remapper_apply(const struct IDRemapper 
*id_remapper,
+                                            struct ID **r_id_ptr,
+                                            IDRemapperApplyOptions options);
+bool BKE_id_remapper_has_mapping_for(const struct IDRemapper *id_remapper, 
uint64_t type_filter);
+void BKE_id_remapper_iter(const struct IDRemapper *id_remapper,
+                          IDRemapperIterFunction func,
+                          void *user_data);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/source/blender/blenkernel/BKE_screen.h 
b/source/blender/blenkernel/BKE_screen.h
index 63f6fca2a9d..c85ae04a492 100644
--- a/source/blender/blenkernel/BKE_screen.h
+++ b/source/blender/blenkernel/BKE_screen.h
@@ -38,6 +38,7 @@ struct BlendLibReader;
 struct BlendWriter;
 struct Header;
 struct ID;
+struct IDRemapper;
 struct LibraryForeachIDData;
 struct ListBase;
 struct Menu;
@@ -117,10 +118,7 @@ typedef struct SpaceType {
   bContextDataCallback context;
 
   /* Used when we want to replace an ID by another (or NULL). */
-  void (*id_remap)(struct ScrArea *area,
-                   struct SpaceLink *sl,
-                   struct ID *old_id,
-                   struct ID *new_id);
+  void (*id_remap)(struct ScrArea *area, struct SpaceLink *sl, const struct 
IDRemapper *mappings);
 
   int (*space_subtype_get)(struct ScrArea *area);
   void (*space_subtype_set)(struct ScrArea *area, int value);
diff --git a/source/blender/blenkernel/CMakeLists.txt 
b/source/blender/blenkernel/CMakeLists.txt
index 41ca8084849..6d6579f49f6 100644
--- a/source/blender/blenkernel/CMakeLists.txt
+++ b/source/blender/blenkernel/CMakeLists.txt
@@ -179,6 +179,7 @@ set(SRC
   intern/lib_id.c
   intern/lib_id_delete.c
   intern/lib_id_eval.c
+  intern/lib_id_remapper.cc
   intern/lib_override.c
   intern/lib_query.c
   intern/lib_remap.c
@@ -823,6 +824,7 @@ if(WITH_GTESTS)
     intern/idprop_serialize_test.cc
     intern/lattice_deform_test.cc
     intern/layer_test.cc
+    intern/lib_id_remapper_test.cc
     intern/lib_id_test.cc
     intern/lib_remap_test.cc
     intern/tracking_test.cc
diff --git a/source/blender/blenkernel/intern/lib_id_delete.c 
b/source/blender/blenkernel/intern/lib_id_delete.c
index 6d2e89187f7..f4dd67cac28 100644
--- a/source/blender/blenkernel/intern/lib_id_delete.c
+++ b/source/blender/blenkernel/intern/lib_id_delete.c
@@ -154,7 +154,10 @@ void BKE_id_free_ex(Main *bmain, void *idv, int flag, 
const bool use_flag_from_i
     }
 
     if (remap_editor_id_reference_cb) {
-      remap_editor_id_reference_cb(id, NULL);
+      struct IDRemapper *remapper = BKE_id_remapper_create();
+      BKE_id_remapper_add(remapper, id, NULL);
+      remap_editor_id_reference_cb(remapper);
+      BKE_id_remapper_free(remapper);
     }
   }
 
@@ -292,32 +295,40 @@ static size_t id_delete(Main *bmain, const bool 
do_tagged_deletion)
      * Note that we go forward here, since we want to check dependencies 
before users
      * (e.g. meshes before objects).
      * Avoids to have to loop twice. */
+    struct IDRemapper *remapper = BKE_id_remapper_create();
     for (i = 0; i < base_count; i++) {
       ListBase *lb = lbarray[i];
       ID *id, *id_next;
+      BKE_id_remapper_clear(remapper);
 
       for (id = lb->first; id; id = id_next) {
         id_next = id->next;
         /* NOTE: in case we delete a library, we also delete all its 
datablocks! */
         if ((id->tag & tag) || (id->lib != NULL && (id->lib->id.tag & tag))) {
           id->tag |= tag;
-
-          /* Will tag 'never NULL' users of this ID too.
-           * Note that we cannot use BKE_libblock_unlink() here, since it 
would ignore indirect
-           * (and proxy!) links, this can lead to nasty crashing here in 
second,
-           * actual deleting loop.
-           * Also, this will also flag users of deleted data that cannot be 
unlinked
-           * (object using deleted obdata, etc.), so that they also get 
deleted. */
-          BKE_libblock_remap_locked(bmain,
-                                    id,
-                                    NULL,
-                                    (ID_REMAP_FLAG_NEVER_NULL_USAGE |
-                                     ID_REMAP_FORCE_NEVER_NULL_USAGE |
-                                     
ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS));
+          BKE_id_remapper_add(remapper, id, NULL);
         }
       }
+
+      if (BKE_id_remapper_is_empty(remapper)) {
+        continue;
+      }
+
+      /* Will tag 'never NULL' users of this ID too.
+       * Note that we cannot use BKE_libblock_unlink() here, since it would 
ignore indirect
+       * (and proxy!) links, this can lead to nasty crashing here in second,
+       * actual deleting loop.
+       * Also, this will also flag users of deleted data that cannot be 
unlinked
+       * (object using deleted obdata, etc.), so that they also get deleted. */
+      BKE_libblock_remap_multiple_locked(bmain,
+                                         remapper,
+                                         (ID_REMAP_FLAG_NEVER_NULL_USAGE |
+                                          ID_REMAP_FORCE_NEVER_NULL_USAGE |
+                                          
ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS));
     }
+    BKE_id_remapper_free(remapper);
   }
+
   BKE_main_unlock(bmain);
 
   /* In usual reversed order, such that all usage of a given ID, even 'never 
NULL' ones,
diff --git a/source/blender/blenkernel/intern/lib_id_remapper.cc 
b/source/blender/blenkernel/intern/lib_id_remapper.cc
new file mode 100644
index 00000000000..c1734c9826a
--- /dev/null
+++ b/source/blender/blenkernel/intern/lib_id_remapper.cc
@@ -0,0 +1,175 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * The Original Code is Copyright (C) 2022 by Blender Foundation.
+ */
+
+#include "DNA_ID.h"
+
+#include "BKE_idtype.h"
+#include "BKE_lib_id.h"
+#include "BKE_lib_remap.h"
+
+#include "MEM_guardedalloc.h"
+
+#include "BLI_map.hh"
+
+using IDTypeFilter = uint64_t;
+
+namespace blender::bke::id::remapper {
+struct IDRemapper {
+ private:
+  Map<ID *, ID *> mappings;
+  IDTypeFilter source_types = 0;
+
+ public:
+  void clear()
+  {
+    mappings.clear();
+    source_types = 0;
+  }
+
+  bool is_empty() const
+  {
+    return mappings.is_empty();
+  }
+
+  void add(ID *old_id, ID *new_id)
+  {
+    BLI_assert(old_id != nullptr);
+    BLI_assert(new_id == nullptr || (GS(old_id->name) == GS(new_id->name)));

@@ Diff output truncated at 10240 characters. @@

_______________________________________________
Bf-blender-cvs mailing list
[email protected]
List details, subscription details or unsubscribe:
https://lists.blender.org/mailman/listinfo/bf-blender-cvs

Reply via email to