From a71a513def202b119328035dbd68e86c2c47f7ac Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Fri, 11 Feb 2022 14:53:08 +0100 Subject: Remap multiple items in referenced data. This patch increases the performance when remapping data. {D13615} introduced a mechanism to remap multiple items in a single go. This patch uses the same mechanism when remapping data inside ID datablocks. Benchmark results when loading the village scene of sprite fright on AMD Ryzen 7 3800X 8-Core Processor Before this patch 115 seconds When patch applied less than 43 seconds There is still some room for improvement by porting relink code. Reviewed By: mont29 Maniphest Tasks: T95279 Differential Revision: https://developer.blender.org/D14043 --- source/blender/blenkernel/intern/lib_remap.c | 319 +++++++++++++++------------ 1 file changed, 179 insertions(+), 140 deletions(-) (limited to 'source/blender/blenkernel/intern/lib_remap.c') diff --git a/source/blender/blenkernel/intern/lib_remap.c b/source/blender/blenkernel/intern/lib_remap.c index a05067e16f6..9329a09f1b6 100644 --- a/source/blender/blenkernel/intern/lib_remap.c +++ b/source/blender/blenkernel/intern/lib_remap.c @@ -51,35 +51,20 @@ void BKE_library_callback_remap_editor_id_reference_set( } typedef struct IDRemap { + eIDRemapType type; Main *bmain; /* Only used to trigger depsgraph updates in the right bmain. */ - ID *old_id; - ID *new_id; + + struct IDRemapper *id_remapper; + /** The ID in which we are replacing old_id by new_id usages. */ ID *id_owner; short flag; - - /* 'Output' data. */ - short status; - /** Number of direct use cases that could not be remapped (e.g.: obdata when in edit mode). */ - int skipped_direct; - /** Number of indirect use cases that could not be remapped. */ - int skipped_indirect; - /** Number of skipped use cases that refcount the data-block. */ - int skipped_refcounted; } IDRemap; /* IDRemap->flag enums defined in BKE_lib.h */ -/* IDRemap->status */ -enum { - /* *** Set by callback. *** */ - ID_REMAP_IS_LINKED_DIRECT = 1 << 0, /* new_id is directly linked in current .blend. */ - ID_REMAP_IS_USER_ONE_SKIPPED = 1 << 1, /* There was some skipped 'user_one' usages of old_id. */ -}; - static void foreach_libblock_remap_callback_skip(const ID *UNUSED(id_owner), - ID **UNUSED(id_ptr), - IDRemap *id_remap_data, + ID **id_ptr, const int cb_flag, const bool is_indirect, const bool is_reference, @@ -87,37 +72,40 @@ static void foreach_libblock_remap_callback_skip(const ID *UNUSED(id_owner), const bool UNUSED(is_obj), const bool is_obj_editmode) { + ID *id = *id_ptr; + BLI_assert(id != NULL); if (is_indirect) { - id_remap_data->skipped_indirect++; + id->runtime.remap.skipped_indirect++; } else if (violates_never_null || is_obj_editmode || is_reference) { - id_remap_data->skipped_direct++; + id->runtime.remap.skipped_direct++; } else { BLI_assert(0); } if (cb_flag & IDWALK_CB_USER) { - id_remap_data->skipped_refcounted++; + id->runtime.remap.skipped_refcounted++; } else if (cb_flag & IDWALK_CB_USER_ONE) { /* No need to count number of times this happens, just a flag is enough. */ - id_remap_data->status |= ID_REMAP_IS_USER_ONE_SKIPPED; + id->runtime.remap.status |= ID_REMAP_IS_USER_ONE_SKIPPED; } } static void foreach_libblock_remap_callback_apply(ID *id_owner, ID *id_self, - ID *old_id, - ID *new_id, ID **id_ptr, IDRemap *id_remap_data, + const struct IDRemapper *mappings, + const IDRemapperApplyOptions id_remapper_options, const int cb_flag, const bool is_indirect, const bool violates_never_null, const bool force_user_refcount) { + ID *old_id = *id_ptr; if (!violates_never_null) { - *id_ptr = new_id; + BKE_id_remapper_apply_ex(mappings, id_ptr, id_remapper_options, id_self); DEG_id_tag_update_ex(id_remap_data->bmain, id_self, ID_RECALC_COPY_ON_WRITE | ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY); @@ -127,6 +115,10 @@ static void foreach_libblock_remap_callback_apply(ID *id_owner, ID_RECALC_COPY_ON_WRITE | ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY); } } + /* Get the new_id pointer. When the mapping is violating never null we should use a NULL + * pointer otherwise the incorrect users are decreased and increased on the same instance. */ + ID *new_id = violates_never_null ? NULL : *id_ptr; + if (cb_flag & IDWALK_CB_USER) { /* NOTE: by default we don't user-count IDs which are not in the main database. * This is because in certain conditions we can have data-blocks in @@ -147,8 +139,8 @@ static void foreach_libblock_remap_callback_apply(ID *id_owner, /* We cannot affect old_id->us directly, LIB_TAG_EXTRAUSER(_SET) * are assumed to be set as needed, that extra user is processed in final handling. */ } - if (!is_indirect) { - id_remap_data->status |= ID_REMAP_IS_LINKED_DIRECT; + if (!is_indirect && new_id) { + new_id->runtime.remap.status |= ID_REMAP_IS_LINKED_DIRECT; } } @@ -164,28 +156,43 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) ID *id_self = cb_data->id_self; ID **id_p = cb_data->id_pointer; IDRemap *id_remap_data = cb_data->user_data; - ID *old_id = id_remap_data->old_id; - ID *new_id = id_remap_data->new_id; /* Those asserts ensure the general sanity of ID tags regarding 'embedded' ID data (root * nodetrees and co). */ BLI_assert(id_owner == id_remap_data->id_owner); BLI_assert(id_self == id_owner || (id_self->flag & LIB_EMBEDDED_DATA) != 0); - if (!old_id) { /* Used to cleanup all IDs used by a specific one. */ - BLI_assert(!new_id); - old_id = *id_p; + /* Early exit when id pointer isn't set. */ + if (*id_p == NULL) { + return IDWALK_RET_NOP; } - /* Early exit when id pointer isn't set to an expected value. */ - if (*id_p == NULL || *id_p != old_id) { - return IDWALK_RET_NOP; + struct IDRemapper *id_remapper = id_remap_data->id_remapper; + IDRemapperApplyOptions id_remapper_options = ID_REMAP_APPLY_DEFAULT; + + /* Used to cleanup all IDs used by a specific one. */ + if (id_remap_data->type == ID_REMAP_TYPE_CLEANUP) { + /* Clearing existing instance to reduce potential lookup times for IDs referencing many other + * IDs. This makes sure that there will only be a single rule in the id_remapper. */ + BKE_id_remapper_clear(id_remapper); + BKE_id_remapper_add(id_remapper, *id_p, NULL); } /* Better remap to NULL than not remapping at all, * then we can handle it as a regular remap-to-NULL case. */ - if ((cb_flag & IDWALK_CB_NEVER_SELF) && (new_id == id_self)) { - new_id = NULL; + if ((cb_flag & IDWALK_CB_NEVER_SELF)) { + id_remapper_options |= ID_REMAP_APPLY_UNMAP_WHEN_REMAPPING_TO_SELF; + } + + const IDRemapperApplyResult expected_mapping_result = BKE_id_remapper_get_mapping_result( + id_remapper, *id_p, id_remapper_options, id_self); + /* Exit, when no modifications will be done; ensuring id->runtime counters won't changed. */ + if (ELEM(expected_mapping_result, + ID_REMAP_RESULT_SOURCE_UNAVAILABLE, + ID_REMAP_RESULT_SOURCE_NOT_MAPPABLE)) { + BLI_assert_msg(id_remap_data->type == ID_REMAP_TYPE_REMAP, + "Cleanup should always do unassign."); + return IDWALK_RET_NOP; } const bool is_reference = (cb_flag & IDWALK_CB_OVERRIDE_LIBRARY_REFERENCE) != 0; @@ -196,7 +203,9 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) * remapped in this situation. */ const bool is_obj_editmode = (is_obj && BKE_object_is_in_editmode((Object *)id_owner) && (id_remap_data->flag & ID_REMAP_FORCE_OBDATA_IN_EDITMODE) == 0); - const bool violates_never_null = ((cb_flag & IDWALK_CB_NEVER_NULL) && (new_id == NULL) && + const bool violates_never_null = ((cb_flag & IDWALK_CB_NEVER_NULL) && + (expected_mapping_result == + ID_REMAP_RESULT_SOURCE_UNASSIGNED) && (id_remap_data->flag & ID_REMAP_FORCE_NEVER_NULL_USAGE) == 0); const bool skip_reference = (id_remap_data->flag & ID_REMAP_SKIP_OVERRIDE_LIBRARY) != 0; const bool skip_never_null = (id_remap_data->flag & ID_REMAP_SKIP_NEVER_NULL_USAGE) != 0; @@ -204,14 +213,13 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) #ifdef DEBUG_PRINT printf( - "In %s (lib %p): Remapping %s (%p) to %s (%p) " + "In %s (lib %p): Remapping %s (%p) remap operation: %s " "(is_indirect: %d, skip_indirect: %d, is_reference: %d, skip_reference: %d)\n", - id->name, - id->lib, - old_id->name, - old_id, - new_id ? new_id->name : "", - new_id, + id_owner->name, + id_owner->lib, + (*id_p)->name, + *id_p, + BKE_id_remapper_result_string(expected_mapping_result), is_indirect, skip_indirect, is_reference, @@ -226,11 +234,11 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) * (otherwise, we follow common NEVER_NULL flags). * (skipped_indirect too). */ if ((violates_never_null && skip_never_null) || - (is_obj_editmode && (((Object *)id_owner)->data == *id_p) && new_id != NULL) || + (is_obj_editmode && (((Object *)id_owner)->data == *id_p) && + (expected_mapping_result == ID_REMAP_RESULT_SOURCE_REMAPPED)) || (skip_indirect && is_indirect) || (is_reference && skip_reference)) { foreach_libblock_remap_callback_skip(id_owner, id_p, - id_remap_data, cb_flag, is_indirect, is_reference, @@ -241,10 +249,10 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) else { foreach_libblock_remap_callback_apply(id_owner, id_self, - old_id, - new_id, id_p, id_remap_data, + id_remapper, + id_remapper_options, cb_flag, is_indirect, violates_never_null, @@ -254,27 +262,47 @@ static int foreach_libblock_remap_callback(LibraryIDLinkCallbackData *cb_data) return IDWALK_RET_NOP; } -static void libblock_remap_data_preprocess(IDRemap *r_id_remap_data) +static void libblock_remap_data_preprocess_ob(Object *ob, + eIDRemapType remap_type, + const struct IDRemapper *id_remapper) +{ + if (ob->type != OB_ARMATURE) { + return; + } + if (ob->pose == NULL) { + return; + } + + const bool is_cleanup_type = remap_type == ID_REMAP_TYPE_CLEANUP; + /* Early exit when mapping, but no armature mappings present. */ + if (!is_cleanup_type && !BKE_id_remapper_has_mapping_for(id_remapper, FILTER_ID_AR)) { + return; + } + + /* Object's pose holds reference to armature bones. sic */ + /* Note that in theory, we should have to bother about linked/non-linked/never-null/etc. + * flags/states. + * Fortunately, this is just a tag, so we can accept to 'over-tag' a bit for pose recalc, + * and avoid another complex and risky condition nightmare like the one we have in + * foreach_libblock_remap_callback(). */ + const IDRemapperApplyResult expected_mapping_result = BKE_id_remapper_get_mapping_result( + id_remapper, ob->data, ID_REMAP_APPLY_DEFAULT, NULL); + if (is_cleanup_type || expected_mapping_result == ID_REMAP_RESULT_SOURCE_REMAPPED) { + ob->pose->flag |= POSE_RECALC; + /* We need to clear pose bone pointers immediately, some code may access those before + * pose is actually recomputed, which can lead to segfault. */ + BKE_pose_clear_pointers(ob->pose); + } +} + +static void libblock_remap_data_preprocess(ID *id_owner, + eIDRemapType remap_type, + const struct IDRemapper *id_remapper) { - switch (GS(r_id_remap_data->id_owner->name)) { + switch (GS(id_owner->name)) { case ID_OB: { - ID *old_id = r_id_remap_data->old_id; - if (!old_id || GS(old_id->name) == ID_AR) { - Object *ob = (Object *)r_id_remap_data->id_owner; - /* Object's pose holds reference to armature bones. sic */ - /* Note that in theory, we should have to bother about linked/non-linked/never-null/etc. - * flags/states. - * Fortunately, this is just a tag, so we can accept to 'over-tag' a bit for pose recalc, - * and avoid another complex and risky condition nightmare like the one we have in - * foreach_libblock_remap_callback(). */ - if (ob->pose && (!old_id || ob->data == old_id)) { - BLI_assert(ob->type == OB_ARMATURE); - ob->pose->flag |= POSE_RECALC; - /* We need to clear pose bone pointers immediately, some code may access those before - * pose is actually recomputed, which can lead to segfault. */ - BKE_pose_clear_pointers(ob->pose); - } - } + Object *ob = (Object *)id_owner; + libblock_remap_data_preprocess_ob(ob, remap_type, id_remapper); break; } default: @@ -369,6 +397,37 @@ static void libblock_remap_data_postprocess_nodetree_update(Main *bmain, ID *new ntreeUpdateAllUsers(bmain, new_id); } +static void libblock_remap_data_update_tags(ID *old_id, ID *new_id, void *user_data) +{ + IDRemap *id_remap_data = user_data; + const int remap_flags = id_remap_data->flag; + if ((remap_flags & ID_REMAP_SKIP_USER_CLEAR) == 0) { + /* XXX We may not want to always 'transfer' fake-user from old to new id... + * Think for now it's desired behavior though, + * we can always add an option (flag) to control this later if needed. */ + if (old_id && (old_id->flag & LIB_FAKEUSER)) { + id_fake_user_clear(old_id); + id_fake_user_set(new_id); + } + + id_us_clear_real(old_id); + } + + if (new_id && (new_id->tag & LIB_TAG_INDIRECT) && + (new_id->runtime.remap.status & ID_REMAP_IS_LINKED_DIRECT)) { + new_id->tag &= ~LIB_TAG_INDIRECT; + new_id->flag &= ~LIB_INDIRECT_WEAK_LINK; + new_id->tag |= LIB_TAG_EXTERN; + } +} + +static void libblock_remap_reset_remapping_status_callback(ID *old_id, + ID *UNUSED(new_id), + void *UNUSED(user_data)) +{ + BKE_libblock_runtime_reset_remapping_status(old_id); +} + /** * Execute the 'data' part of the remapping (that is, all ID pointers from other ID data-blocks). * @@ -392,35 +451,33 @@ static void libblock_remap_data_postprocess_nodetree_update(Main *bmain, ID *new * (uselful to retrieve info about remapping process). */ ATTR_NONNULL(1) -static void libblock_remap_data( - Main *bmain, ID *id, ID *old_id, ID *new_id, const short remap_flags, IDRemap *r_id_remap_data) +static void libblock_remap_data(Main *bmain, + ID *id, + eIDRemapType remap_type, + struct IDRemapper *id_remapper, + const short remap_flags) { - IDRemap id_remap_data; + IDRemap id_remap_data = {0}; const int foreach_id_flags = ((remap_flags & ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS) != 0 ? IDWALK_DO_INTERNAL_RUNTIME_POINTERS : IDWALK_NOP); - if (r_id_remap_data == NULL) { - r_id_remap_data = &id_remap_data; - } - r_id_remap_data->bmain = bmain; - r_id_remap_data->old_id = old_id; - r_id_remap_data->new_id = new_id; - r_id_remap_data->id_owner = NULL; - r_id_remap_data->flag = remap_flags; - r_id_remap_data->status = 0; - r_id_remap_data->skipped_direct = 0; - r_id_remap_data->skipped_indirect = 0; - r_id_remap_data->skipped_refcounted = 0; + id_remap_data.id_remapper = id_remapper; + id_remap_data.type = remap_type; + id_remap_data.bmain = bmain; + id_remap_data.id_owner = NULL; + id_remap_data.flag = remap_flags; + + BKE_id_remapper_iter(id_remapper, libblock_remap_reset_remapping_status_callback, NULL); if (id) { #ifdef DEBUG_PRINT printf("\tchecking id %s (%p, %p)\n", id->name, id, id->lib); #endif - r_id_remap_data->id_owner = id; - libblock_remap_data_preprocess(r_id_remap_data); + id_remap_data.id_owner = id; + libblock_remap_data_preprocess(id_remap_data.id_owner, remap_type, id_remapper); BKE_library_foreach_ID_link( - NULL, id, foreach_libblock_remap_callback, (void *)r_id_remap_data, foreach_id_flags); + NULL, id, foreach_libblock_remap_callback, &id_remap_data, foreach_id_flags); } else { /* Note that this is a very 'brute force' approach, @@ -429,48 +486,28 @@ static void libblock_remap_data( ID *id_curr; FOREACH_MAIN_ID_BEGIN (bmain, id_curr) { - if (BKE_library_id_can_use_idtype(id_curr, GS(old_id->name))) { - /* Note that we cannot skip indirect usages of old_id here (if requested), - * we still need to check it for the user count handling... - * XXX No more true (except for debug usage of those skipping counters). */ - r_id_remap_data->id_owner = id_curr; - libblock_remap_data_preprocess(r_id_remap_data); - BKE_library_foreach_ID_link(NULL, - id_curr, - foreach_libblock_remap_callback, - (void *)r_id_remap_data, - foreach_id_flags); + const uint64_t can_use_filter_id = BKE_library_id_can_use_filter_id(id_curr); + const bool has_mapping = BKE_id_remapper_has_mapping_for(id_remapper, can_use_filter_id); + + /* Continue when id_remapper doesn't have any mappings that can be used by id_curr. */ + if (!has_mapping) { + continue; } - } - FOREACH_MAIN_ID_END; - } - if ((remap_flags & ID_REMAP_SKIP_USER_CLEAR) == 0) { - /* XXX We may not want to always 'transfer' fake-user from old to new id... - * Think for now it's desired behavior though, - * we can always add an option (flag) to control this later if needed. */ - if (old_id && (old_id->flag & LIB_FAKEUSER)) { - id_fake_user_clear(old_id); - id_fake_user_set(new_id); + /* Note that we cannot skip indirect usages of old_id + * here (if requested), we still need to check it for the + * user count handling... + * XXX No more true (except for debug usage of those + * skipping counters). */ + id_remap_data.id_owner = id_curr; + libblock_remap_data_preprocess(id_remap_data.id_owner, remap_type, id_remapper); + BKE_library_foreach_ID_link( + NULL, id_curr, foreach_libblock_remap_callback, &id_remap_data, foreach_id_flags); } - - id_us_clear_real(old_id); - } - - if (new_id && (new_id->tag & LIB_TAG_INDIRECT) && - (r_id_remap_data->status & ID_REMAP_IS_LINKED_DIRECT)) { - new_id->tag &= ~LIB_TAG_INDIRECT; - new_id->flag &= ~LIB_INDIRECT_WEAK_LINK; - new_id->tag |= LIB_TAG_EXTERN; + FOREACH_MAIN_ID_END; } -#ifdef DEBUG_PRINT - printf("%s: %d occurrences skipped (%d direct and %d indirect ones)\n", - __func__, - r_id_remap_data->skipped_direct + r_id_remap_data->skipped_indirect, - r_id_remap_data->skipped_direct, - r_id_remap_data->skipped_indirect); -#endif + BKE_id_remapper_iter(id_remapper, libblock_remap_data_update_tags, &id_remap_data); } typedef struct LibblockRemapMultipleUserData { @@ -484,32 +521,25 @@ static void libblock_remap_foreach_idpair_cb(ID *old_id, ID *new_id, void *user_ Main *bmain = data->bmain; const short remap_flags = data->remap_flags; - IDRemap id_remap_data; - int skipped_direct, skipped_refcounted; - BLI_assert(old_id != NULL); BLI_assert((new_id == NULL) || GS(old_id->name) == GS(new_id->name)); BLI_assert(old_id != new_id); - libblock_remap_data(bmain, NULL, old_id, new_id, remap_flags, &id_remap_data); - if (free_notifier_reference_cb) { free_notifier_reference_cb(old_id); } - skipped_direct = id_remap_data.skipped_direct; - skipped_refcounted = id_remap_data.skipped_refcounted; - if ((remap_flags & ID_REMAP_SKIP_USER_CLEAR) == 0) { /* If old_id was used by some ugly 'user_one' stuff (like Image or Clip editors...), and user * count has actually been incremented for that, we have to decrease once more its user * count... unless we had to skip some 'user_one' cases. */ if ((old_id->tag & LIB_TAG_EXTRAUSER_SET) && - !(id_remap_data.status & ID_REMAP_IS_USER_ONE_SKIPPED)) { + !(old_id->runtime.remap.status & ID_REMAP_IS_USER_ONE_SKIPPED)) { id_us_clear_real(old_id); } } + const int skipped_refcounted = old_id->runtime.remap.skipped_refcounted; if (old_id->us - skipped_refcounted < 0) { CLOG_ERROR(&LOG, "Error in remapping process from '%s' (%p) to '%s' (%p): " @@ -522,6 +552,7 @@ static void libblock_remap_foreach_idpair_cb(ID *old_id, ID *new_id, void *user_ BLI_assert(0); } + const int skipped_direct = old_id->runtime.remap.skipped_direct; if (skipped_direct == 0) { /* old_id is assumed to not be used directly anymore... */ if (old_id->lib && (old_id->tag & LIB_TAG_EXTERN)) { @@ -567,10 +598,12 @@ static void libblock_remap_foreach_idpair_cb(ID *old_id, ID *new_id, void *user_ /* Full rebuild of DEG! */ DEG_relations_tag_update(bmain); + + BKE_libblock_runtime_reset_remapping_status(old_id); } void BKE_libblock_remap_multiple_locked(Main *bmain, - const struct IDRemapper *mappings, + struct IDRemapper *mappings, const short remap_flags) { if (BKE_id_remapper_is_empty(mappings)) { @@ -578,9 +611,12 @@ void BKE_libblock_remap_multiple_locked(Main *bmain, return; } - LibBlockRemapMultipleUserData user_data; + libblock_remap_data(bmain, NULL, ID_REMAP_TYPE_REMAP, mappings, remap_flags); + + LibBlockRemapMultipleUserData user_data = {0}; user_data.bmain = bmain; user_data.remap_flags = remap_flags; + BKE_id_remapper_iter(mappings, libblock_remap_foreach_idpair_cb, &user_data); /* We assume editors do not hold references to their IDs... This is false in some cases @@ -613,9 +649,7 @@ void BKE_libblock_remap(Main *bmain, void *old_idv, void *new_idv, const short r BKE_main_unlock(bmain); } -void BKE_libblock_remap_multiple(Main *bmain, - const struct IDRemapper *mappings, - const short remap_flags) +void BKE_libblock_remap_multiple(Main *bmain, struct IDRemapper *mappings, const short remap_flags) { BKE_main_lock(bmain); @@ -658,17 +692,22 @@ void BKE_libblock_relink_ex( ID *new_id = new_idv; /* No need to lock here, we are only affecting given ID, not bmain database. */ + struct IDRemapper *id_remapper = BKE_id_remapper_create(); + eIDRemapType remap_type = ID_REMAP_TYPE_REMAP; BLI_assert(id); if (old_id) { BLI_assert((new_id == NULL) || GS(old_id->name) == GS(new_id->name)); BLI_assert(old_id != new_id); + BKE_id_remapper_add(id_remapper, old_id, new_id); } else { BLI_assert(new_id == NULL); + remap_type = ID_REMAP_TYPE_CLEANUP; } - libblock_remap_data(bmain, id, old_id, new_id, remap_flags, NULL); + libblock_remap_data(bmain, id, remap_type, id_remapper, remap_flags); + BKE_id_remapper_free(id_remapper); /* Some after-process updates. * This is a bit ugly, but cannot see a way to avoid it. -- cgit v1.2.3