Merge branch 'master' into id-ensure-unique-memory-address

Disclaimer: this should be considered as an engeneering, explanatory patch.

Rational: for undo work, we are now manipulating IDs from potentially many different 'memory realms' (in contrast to current undo system, where, since we always reallocate the whole main database at each undo step, we only ever have to deal with two, the old one from the memfile, and the new one from re-read data from that memfile).

So far, new undo system has used ID names-based search to remap old to new pointers, but this is slower, less safe, and forces us to do full undo barrier e.g. when renaming an ID.

Idea of this patch is instead to ensure that within a given editing session, we never ever allocate a data-block at the same address twice. That way, we can consider ID pointers as valid UIDs withing that scope, and use their values to handle remapping in udo steps even in case an ID has had several memory addresses in the history of the editing session.

This patches does two things:
# Add a way to ensure a newly allocated ID gets a memory address never used before for any ID.
# Store the history of all memory addresses ever used by a given ID.

Note that point 1 above is currently using a fairly simple and naive solution, but also quiet innefficient (just allocating again and again until we get a 'free' address), we can imagine a much more efficient solution if we go deeper in MEM_ allocator system itself. Not sure if it is worth it though, as with new undo system actual re-allocation of a same ID at a different address should be fairly rare (i.e. history of used/forbidden addresses should remain quiet short).

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

source/blender/blenkernel/BKE_lib_id.h

@@ -62,7 +62,7 @@ struct PropertyRNA;
 struct bContext;
 
 size_t BKE_libblock_get_alloc_info(short type, const char **name);
-void *BKE_libblock_alloc_notest(short type) ATTR_WARN_UNUSED_RESULT;
+void *BKE_libblock_alloc_notest(struct Main *bmain, short type) ATTR_WARN_UNUSED_RESULT;
 void *BKE_libblock_alloc(struct Main *bmain, short type, const char *name, const int flag)
     ATTR_WARN_UNUSED_RESULT;
 void BKE_libblock_init_empty(struct ID *id) ATTR_NONNULL(1);

source/blender/blenkernel/BKE_main.h

@@ -51,6 +51,7 @@ struct GHash;
 struct GSet;
 struct ImBuf;
 struct Library;
+struct LinkNode;
 struct MainLock;
 
 /* Blender thumbnail, as written on file (width, height, and data as char RGBA). */
@@ -100,6 +101,10 @@ typedef struct Main {
    */
   char is_memfile_undo_flush_needed;
 
+  struct GHash *used_id_memhash;
+  struct LinkNode *used_id_memhash_history_chains;
+  short used_id_memhash_tag;
+
   BlendThumbnail *blen_thumb;
 
   struct Library *curlib;
@@ -150,12 +155,32 @@ typedef struct Main {
   struct MainLock *lock;
 } Main;
 
+/* Main.used_id_memory_pointers_tag */
+enum {
+  MAIN_IDMEMHASH_OWNER = 1 << 0,
+};
+
 struct Main *BKE_main_new(void);
 void BKE_main_free(struct Main *mainvar);
 
 void BKE_main_lock(struct Main *bmain);
 void BKE_main_unlock(struct Main *bmain);
 
+void BKE_main_idmemhash_ensure(struct Main *bmain);
+void BKE_main_idmemhash_release(struct Main *bmain);
+void BKE_main_idmemhash_transfer_ownership(struct Main *bmain_dst, struct Main *bmain_src);
+void BKE_main_idmemhash_usefrom(struct Main *bmain_user, struct Main *bmain_src);
+bool BKE_main_idmemhash_register_id(struct Main *bmain, void *old_vmemh, struct ID *id);
+struct ID *BKE_main_idmemhash_lookup_id(struct Main *bmain,
+                                        void *vmemh,
+                                        struct LinkNode **r_used_id_chain);
+void *BKE_main_idmemhash_unique_alloc(struct Main *bmain,
+                                      void *old_vmemh,
+                                      void *(*alloc_cb)(size_t len, const char *str),
+                                      size_t size,
+                                      const char *message);
+void *BKE_main_idmemhash_unique_realloc(struct Main *bmain, void *old_vmemh, void *vmemh);
+
 void BKE_main_relations_create(struct Main *bmain, const short flag);
 void BKE_main_relations_free(struct Main *bmain);
 

source/blender/blenkernel/intern/blender.c

@@ -127,6 +127,7 @@ void BKE_blender_globals_init(void)
   U.savetime = 1;
 
   G_MAIN = BKE_main_new();
+  BKE_main_idmemhash_ensure(G_MAIN);
 
   strcpy(G.ima, "//");
 

source/blender/blenkernel/intern/lib_id.c

@@ -1193,12 +1193,12 @@ size_t BKE_libblock_get_alloc_info(short type, const char **name)
  * Allocates and returns memory of the right size for the specified block type,
  * initialized to zero.
  */
-void *BKE_libblock_alloc_notest(short type)
+void *BKE_libblock_alloc_notest(Main *bmain, short type)
 {
   const char *name;
   size_t size = BKE_libblock_get_alloc_info(type, &name);
   if (size != 0) {
-    return MEM_callocN(size, name);
+    return BKE_main_idmemhash_unique_alloc(bmain, NULL, MEM_callocN, size, name);
   }
   BLI_assert(!"Request to allocate unknown data type");
   return NULL;
@@ -1214,7 +1214,7 @@ void *BKE_libblock_alloc(Main *bmain, short type, const char *name, const int fl
 {
   BLI_assert((flag & LIB_ID_CREATE_NO_ALLOCATE) == 0);
 
-  ID *id = BKE_libblock_alloc_notest(type);
+  ID *id = BKE_libblock_alloc_notest(bmain, type);
 
   if (id) {
     if ((flag & LIB_ID_CREATE_NO_MAIN) != 0) {

source/blender/blenkernel/intern/main.c

@@ -29,6 +29,7 @@
 
 #include "BLI_blenlib.h"
 #include "BLI_ghash.h"
+#include "BLI_linklist.h"
 #include "BLI_mempool.h"
 #include "BLI_threads.h"
 
@@ -194,6 +195,8 @@ void BKE_main_free(Main *mainvar)
     BKE_main_relations_free(mainvar);
   }
 
+  BKE_main_idmemhash_release(mainvar);
+
   BLI_spin_end((SpinLock *)mainvar->lock);
   MEM_freeN(mainvar->lock);
   MEM_freeN(mainvar);
@@ -209,6 +212,146 @@ void BKE_main_unlock(struct Main *bmain)
   BLI_spin_unlock((SpinLock *)bmain->lock);
 }
 
+void BKE_main_idmemhash_ensure(Main *bmain)
+{
+  if (bmain->used_id_memhash == NULL || (bmain->used_id_memhash_tag & MAIN_IDMEMHASH_OWNER) == 0) {
+    bmain->used_id_memhash = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__);
+    bmain->used_id_memhash_history_chains = NULL;
+    bmain->used_id_memhash_tag |= MAIN_IDMEMHASH_OWNER;
+  }
+}
+
+static void main_idmemhash_history_chains_free(void *linkv)
+{
+  LinkNode *link = linkv;
+  BLI_linklist_free(link, NULL);
+}
+
+void BKE_main_idmemhash_release(Main *bmain)
+{
+  if (bmain->used_id_memhash != NULL) {
+    if ((bmain->used_id_memhash_tag & MAIN_IDMEMHASH_OWNER) != 0) {
+      BLI_ghash_free(bmain->used_id_memhash, NULL, NULL);
+      BLI_linklist_free(bmain->used_id_memhash_history_chains, main_idmemhash_history_chains_free);
+    }
+    bmain->used_id_memhash = NULL;
+    bmain->used_id_memhash_history_chains = NULL;
+    bmain->used_id_memhash_tag &= ~MAIN_IDMEMHASH_OWNER;
+  }
+}
+
+void BKE_main_idmemhash_transfer_ownership(Main *bmain_dst, Main *bmain_src)
+{
+  BKE_main_idmemhash_release(bmain_dst);
+
+  BLI_assert(bmain_src->used_id_memhash != NULL);
+  BLI_assert(bmain_src->used_id_memhash_tag & MAIN_IDMEMHASH_OWNER);
+
+  bmain_dst->used_id_memhash = bmain_src->used_id_memhash;
+  bmain_dst->used_id_memhash_history_chains = bmain_src->used_id_memhash_history_chains;
+  bmain_dst->used_id_memhash_tag |= MAIN_IDMEMHASH_OWNER;
+  bmain_src->used_id_memhash_tag &= ~MAIN_IDMEMHASH_OWNER;
+}
+
+void BKE_main_idmemhash_usefrom(Main *bmain_user, Main *bmain_src)
+{
+  BKE_main_idmemhash_release(bmain_user);
+
+  BLI_assert(bmain_src->used_id_memhash != NULL);
+  bmain_user->used_id_memhash = bmain_src->used_id_memhash;
+  bmain_user->used_id_memhash_history_chains = bmain_src->used_id_memhash_history_chains;
+}
+
+/**
+ * @return true if the ID was successfully added to the memset, false if it already existed.
+ */
+bool BKE_main_idmemhash_register_id(Main *bmain, void *old_vmemh, ID *id)
+{
+  BLI_assert(bmain->used_id_memhash != NULL);
+  BLI_assert(old_vmemh != id);
+  void **val;
+  if (!BLI_ghash_ensure_p(bmain->used_id_memhash, id, &val)) {
+    if (old_vmemh != NULL) {
+      LinkNode **chain_hook = (LinkNode **)BLI_ghash_lookup_p(bmain->used_id_memhash, old_vmemh);
+      BLI_assert(chain_hook != NULL);
+      if (*chain_hook == NULL) {
+        /* That datablock only ever had one address so far, we need to initialize its addresses
+         * history chain. */
+        *chain_hook = MEM_callocN(sizeof(**chain_hook), __func__);
+        LinkNode *old_id_entry = MEM_mallocN(sizeof(*old_id_entry), __func__);
+        old_id_entry->link = old_vmemh;
+        old_id_entry->next = NULL;
+        BLI_linklist_prepend_nlink(
+            &bmain->used_id_memhash_history_chains, old_id_entry, *chain_hook);
+      }
+      LinkNode *curr_id_entry = MEM_mallocN(sizeof(*curr_id_entry), __func__);
+      BLI_linklist_prepend_nlink((LinkNode **)&(*chain_hook)->link, id, curr_id_entry);
+      *val = *chain_hook;
+    }
+    else {
+      *val = NULL;
+    }
+    return true;
+  }
+  return false;
+}
+
+/**
+ * Lookup a random ID memory address, and return its last known valid instance, and the linked list
+ * of all its known addresses so far.
+ *
+ * \param r_used_id_chain If not NULL, and that address has had several previous instances, the
+ * linked list storing all of those.
+ * \return The last known instance address matching given \a vmemh pointer, or vmemh itself if it
+ * is unknown.
+ */
+ID *BKE_main_idmemhash_lookup_id(Main *bmain, void *vmemh, LinkNode **r_used_id_chain)
+{
+  LinkNode *used_id_chain_hook = BLI_ghash_lookup(bmain->used_id_memhash, vmemh);
+  LinkNode *used_id_chain = used_id_chain_hook ? used_id_chain_hook->link : NULL;
+  if (r_used_id_chain != NULL) {
+    *r_used_id_chain = used_id_chain;
+  }
+  /* The last valid address should always be the first one in the chain. */
+  return used_id_chain != NULL ? used_id_chain->link : vmemh;
+}
+
+void *BKE_main_idmemhash_unique_alloc(Main *bmain,
+                                      void *old_vmemh,
+                                      void *(*alloc_cb)(size_t len, const char *str),
+                                      size_t size,
+                                      const char *message)
+{
+  void *id_mem = alloc_cb(size, message);
+  if (bmain != NULL && bmain->used_id_memhash != NULL) {
+    ListBase generated_ids = {.first = NULL};
+    int count = 0;
+    while (UNLIKELY(!BKE_main_idmemhash_register_id(bmain, old_vmemh, id_mem))) {
+      printf("Allocating ID re-used memory address %p, trying again (%d)...\n", id_mem, ++count);
+      BLI_addtail(&generated_ids, id_mem);
+      id_mem = alloc_cb(size, message);
+    }
+    BLI_freelistN(&generated_ids);
+  }
+  return id_mem;
+}
+
+void *BKE_main_idmemhash_unique_realloc(Main *bmain, void *old_vmemh, void *vmemh)
+{
+  void *id_mem = MEM_dupallocN(vmemh);
+  if (bmain != NULL && bmain->used_id_memhash != NULL) {
+    ListBase generated_ids = {.first = NULL};
+    int count = 0;
+    while (UNLIKELY(!BKE_main_idmemhash_register_id(bmain, old_vmemh, id_mem))) {
+      printf("Allocating ID re-used memory address %p, trying again (%d)...\n", id_mem, ++count);
+      BLI_addtail(&generated_ids, id_mem);
+      id_mem = MEM_dupallocN(id_mem);
+    }
+    BLI_freelistN(&generated_ids);
+  }
+  return id_mem;
+}
+
 static int main_relations_create_idlink_cb(LibraryIDLinkCallbackData *cb_data)
 {
   MainIDRelations *rel = cb_data->user_data;

source/blender/blenloader/intern/readfile.c

@@ -504,6 +504,10 @@ static void add_main_to_main(Main *mainvar, Main *from)
   ListBase *lbarray[MAX_LIBARRAY], *fromarray[MAX_LIBARRAY];
   int a;
 
+  if (!ELEM(from->used_id_memhash, NULL, mainvar->used_id_memhash)) {
+    BLI_assert(ELEM(from->used_id_memhash, NULL, mainvar->used_id_memhash));
+  }
+
   set_listbasepointers(mainvar, lbarray);
   a = set_listbasepointers(from, fromarray);
   while (a--) {
@@ -561,6 +565,7 @@ void blo_split_main(ListBase *mainlist, Main *main)
   int i = 0;
   for (Library *lib = main->libraries.first; lib; lib = lib->id.next, i++) {
     Main *libmain = BKE_main_new();
+    BKE_main_idmemhash_usefrom(libmain, main);
     libmain->curlib = lib;
     libmain->versionfile = lib->versionfile;
     libmain->subversionfile = lib->subversionfile;
@@ -671,6 +676,7 @@ static Main *blo_find_main(FileData *fd, const char *filepath, const char *relab
   }
 
   m = BKE_main_new();
+  BKE_main_idmemhash_usefrom(m, mainlist->first);
   BLI_addtail(mainlist, m);
 
   /* Add library data-block itself to 'main' Main, since libraries are **never** linked data.
@@ -8204,6 +8210,7 @@ static void direct_link_library(FileData *fd, Library *lib, Main *main)
 
   /* new main */
   newmain = BKE_main_new();
+  BKE_main_idmemhash_usefrom(newmain, fd->mainlist->first);
   BLI_addtail(fd->mainlist, newmain);
   newmain->curlib = lib;
 
@@ -8788,7 +8795,7 @@ static void direct_link_linestyle(FileData *fd, FreestyleLineStyle *linestyle)
 static ID *create_placeholder(Main *mainvar, const short idcode, const char *idname, const int tag)
 {
   ListBase *lb = which_libbase(mainvar, idcode);
-  ID *ph_id = BKE_libblock_alloc_notest(idcode);
+  ID *ph_id = BKE_libblock_alloc_notest(mainvar, idcode);
 
   *((short *)ph_id->name) = idcode;
   BLI_strncpy(ph_id->name + 2, idname, sizeof(ph_id->name) - 2);
@@ -8967,6 +8974,10 @@ static BHead *read_libblock(FileData *fd,
            * like it used to be. */
           BLI_remlink(fd->old_mainlist, libmain);
           BLI_remlink_safe(&oldmain->libraries, libmain->curlib);
+          /* We also need to transfer the unique id storage system, since that libmain now belongs
+           * to the new Main database. */
+          BKE_main_idmemhash_release(libmain);
+          BKE_main_idmemhash_usefrom(libmain, fd->mainlist->first);
           BLI_addtail(fd->mainlist, libmain);
           BLI_addtail(&main->libraries, libmain->curlib);
 
@@ -9035,6 +9046,10 @@ static BHead *read_libblock(FileData *fd,
   id->newid = NULL; /* Needed because .blend may have been saved with crap value here... */
   id->orig_id = NULL;
 
+  const bool is_id_memaddress_unique = BKE_main_idmemhash_register_id(main, NULL, id);
+  /* Note: this is likely to fail at some point with current undo/redo code! */
+  BLI_assert(is_id_memaddress_unique);
+
   /* this case cannot be direct_linked: it's just the ID part */
   if (bhead->code == ID_LINK_PLACEHOLDER) {
     /* That way, we know which data-lock needs do_versions (required currently for linking). */
@@ -9643,6 +9658,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
   bfd = MEM_callocN(sizeof(BlendFileData), "blendfiledata");
 
   bfd->main = BKE_main_new();
+  BKE_main_idmemhash_ensure(bfd->main);
   bfd->main->versionfile = fd->fileversion;
 
   bfd->type = BLENFILETYPE_BLEND;
@@ -11405,6 +11421,7 @@ static void split_main_newid(Main *mainptr, Main *main_newid)
   main_newid->subversionfile = mainptr->subversionfile;
   BLI_strncpy(main_newid->name, mainptr->name, sizeof(main_newid->name));
   main_newid->curlib = mainptr->curlib;
+  BKE_main_idmemhash_usefrom(main_newid, mainptr);
 
   ListBase *lbarray[MAX_LIBARRAY];
   ListBase *lbarray_newid[MAX_LIBARRAY];

source/blender/depsgraph/intern/node/deg_node_id.cc

@@ -136,7 +136,7 @@ void IDNode::init_copy_on_write(ID *id_cow_hint)
     }
   }
   else if (deg_copy_on_write_is_needed(id_orig)) {
-    id_cow = (ID *)BKE_libblock_alloc_notest(GS(id_orig->name));
+    id_cow = (ID *)BKE_libblock_alloc_notest(NULL, GS(id_orig->name));
     DEG_COW_PRINT(
         "Create shallow copy for %s: id_orig=%p id_cow=%p\n", id_orig->name, id_orig, id_cow);
     deg_tag_copy_on_write_id(id_cow, id_orig);