Commit: 6374644fd11797806ab2e92341e5e7d32e94067b Author: Jacques Lucke Date: Tue Sep 29 12:29:01 2020 +0200 Branches: master https://developer.blender.org/rB6374644fd11797806ab2e92341e5e7d32e94067b
DNA: optimize struct reconstruction by doing some preprocessing When loading large files that are more than a couple weeks old (such that DNA has changed in that time), a significant amount of time is spent in `DNA_struct_reconstruct`. This function takes a struct in the old layout and creates a struct in the new layout from it. This was slow because it was computing the diff between the struct layouts every time a struct is updated. Now the steps for the struct reconstruction is computed only once per struct. This information is then used to actually reconstruct all structs that changed. I measured about 10-20% speedup when loading Spring files. E.g. `10.6s -> 8.7s` for `06_055_A.anim.blend` in BKE_blendfile_read`. This percentage varies a lot based on the number of blocks that have to be reconstructed and how much DNA has changed since they have been written. In none of my tests was the new code slower than the old code. Reviewers: campbellbarton Differential Revision: https://developer.blender.org/D8959 =================================================================== M source/blender/blenloader/intern/readfile.c M source/blender/blenloader/intern/readfile.h M source/blender/makesdna/DNA_genfile.h M source/blender/makesdna/DNA_sdna_types.h M source/blender/makesdna/intern/dna_genfile.c =================================================================== diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index 19fab73b259..bd944ac32ac 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -779,7 +779,7 @@ static void bh4_from_bh8(BHead *bhead, BHead8 *bhead8, int do_endian_swap) * 0x0000000000000000000012345678 would become 0x12345678000000000000000000000000 */ if (do_endian_swap) { - BLI_endian_switch_int64(&bhead8->old); + BLI_endian_switch_uint64(&bhead8->old); } /* this patch is to avoid a long long being read from not-eight aligned positions @@ -1114,6 +1114,8 @@ static bool read_file_dna(FileData *fd, const char **r_error_message) if (fd->filesdna) { blo_do_versions_dna(fd->filesdna, fd->fileversion, subversion); fd->compflags = DNA_struct_get_compareflags(fd->filesdna, fd->memsdna); + fd->reconstruct_info = DNA_reconstruct_info_create( + fd->filesdna, fd->memsdna, fd->compflags); /* used to retrieve ID names from (bhead+1) */ fd->id_name_offs = DNA_elem_offset(fd->filesdna, "ID", "char", "name[]"); @@ -1610,6 +1612,9 @@ void blo_filedata_free(FileData *fd) if (fd->compflags) { MEM_freeN((void *)fd->compflags); } + if (fd->reconstruct_info) { + DNA_reconstruct_info_free(fd->reconstruct_info); + } if (fd->datamap) { oldnewmap_free(fd->datamap); @@ -2177,8 +2182,7 @@ static void *read_struct(FileData *fd, BHead *bh, const char *blockname) } } #endif - temp = DNA_struct_reconstruct( - fd->memsdna, fd->filesdna, fd->compflags, bh->SDNAnr, bh->nr, (bh + 1)); + temp = DNA_struct_reconstruct(fd->reconstruct_info, bh->SDNAnr, bh->nr, (bh + 1)); } else { /* SDNA_CMP_EQUAL */ @@ -9135,7 +9139,7 @@ static void convert_pointer_array_32_to_64(BlendDataReader *UNUSED(reader), const uint32_t *src, uint64_t *dst) { - /* Match pointer conversion rules from bh8_from_bh4 and cast_pointer. */ + /* Match pointer conversion rules from bh8_from_bh4 and cast_pointer_32_to_64. */ for (int i = 0; i < array_size; i++) { dst[i] = src[i]; } diff --git a/source/blender/blenloader/intern/readfile.h b/source/blender/blenloader/intern/readfile.h index d43f7ded50e..2ddf96a2d47 100644 --- a/source/blender/blenloader/intern/readfile.h +++ b/source/blender/blenloader/intern/readfile.h @@ -106,6 +106,7 @@ typedef struct FileData { const struct SDNA *memsdna; /** Array of #eSDNA_StructCompare. */ const char *compflags; + struct DNA_ReconstructInfo *reconstruct_info; int fileversion; /** Used to retrieve ID names from (bhead+1). */ diff --git a/source/blender/makesdna/DNA_genfile.h b/source/blender/makesdna/DNA_genfile.h index 1cdaba81ffa..b09b6dcd8e4 100644 --- a/source/blender/makesdna/DNA_genfile.h +++ b/source/blender/makesdna/DNA_genfile.h @@ -78,6 +78,8 @@ enum eSDNA_StructCompare { /* Struct is different in some way * (needs to be copied/converted field by field) */ SDNA_CMP_NOT_EQUAL = 2, + /* This is only used temporarily by #DNA_struct_get_compareflags. */ + SDNA_CMP_UNKNOWN = 3, }; struct SDNA *DNA_sdna_from_data(const void *data, @@ -93,13 +95,17 @@ void DNA_sdna_current_init(void); const struct SDNA *DNA_sdna_current_get(void); void DNA_sdna_current_free(void); +struct DNA_ReconstructInfo; +struct DNA_ReconstructInfo *DNA_reconstruct_info_create(const struct SDNA *oldsdna, + const struct SDNA *newsdna, + const char *compare_flags); +void DNA_reconstruct_info_free(struct DNA_ReconstructInfo *reconstruct_info); + int DNA_struct_find_nr_ex(const struct SDNA *sdna, const char *str, unsigned int *index_last); int DNA_struct_find_nr(const struct SDNA *sdna, const char *str); void DNA_struct_switch_endian(const struct SDNA *oldsdna, int oldSDNAnr, char *data); const char *DNA_struct_get_compareflags(const struct SDNA *sdna, const struct SDNA *newsdna); -void *DNA_struct_reconstruct(const struct SDNA *newsdna, - const struct SDNA *oldsdna, - const char *compflags, +void *DNA_struct_reconstruct(const struct DNA_ReconstructInfo *reconstruct_info, int oldSDNAnr, int blocks, const void *data); diff --git a/source/blender/makesdna/DNA_sdna_types.h b/source/blender/makesdna/DNA_sdna_types.h index 3af12ae791f..05f86e32961 100644 --- a/source/blender/makesdna/DNA_sdna_types.h +++ b/source/blender/makesdna/DNA_sdna_types.h @@ -108,13 +108,13 @@ typedef struct BHead { # typedef struct BHead4 { int code, len; - int old; + uint old; int SDNAnr, nr; } BHead4; # # typedef struct BHead8 { int code, len; - int64_t old; + uint64_t old; int SDNAnr, nr; } BHead8; diff --git a/source/blender/makesdna/intern/dna_genfile.c b/source/blender/makesdna/intern/dna_genfile.c index b140a847188..74f587fe032 100644 --- a/source/blender/makesdna/intern/dna_genfile.c +++ b/source/blender/makesdna/intern/dna_genfile.c @@ -611,34 +611,89 @@ void DNA_sdna_current_free(void) /* ******************* HANDLE DNA ***************** */ /** - * Used by #DNA_struct_get_compareflags (below) to recursively mark all structs - * containing a field of type structnr as changed between old and current SDNAs. + * This function changes compare_flags[old_struct_index] from SDNA_CMP_UNKNOWN to something else. + * It might call itself recursively. */ -static void recurs_test_compflags(const SDNA *sdna, char *compflags, int structnr) +static void set_compare_flags_for_struct(const SDNA *oldsdna, + const SDNA *newsdna, + char *compare_flags, + const int old_struct_index) { - /* check all structs, test if it's inside another struct */ - const int typenr = sdna->structs[structnr]->type; - - for (int a = 0; a < sdna->structs_len; a++) { - if (a != structnr && compflags[a] == SDNA_CMP_EQUAL) { - SDNA_Struct *struct_info = sdna->structs[a]; - for (int b = 0; b < struct_info->members_len; b++) { - SDNA_StructMember *member = &struct_info->members[b]; - if (member->type == typenr) { - const char *member_name = sdna->names[member->name]; - if (!ispointer(member_name)) { - compflags[a] = SDNA_CMP_NOT_EQUAL; - recurs_test_compflags(sdna, compflags, a); - } + if (compare_flags[old_struct_index] != SDNA_CMP_UNKNOWN) { + /* This flag has been initialized already. */ + return; + } + + SDNA_Struct *old_struct = oldsdna->structs[old_struct_index]; + const char *struct_name = oldsdna->types[old_struct->type]; + + const int new_struct_index = DNA_struct_find_nr(newsdna, struct_name); + if (new_struct_index == -1) { + /* Didn't find a matching new struct, so it has been removed. */ + compare_flags[old_struct_index] = SDNA_CMP_REMOVED; + return; + } + + SDNA_Struct *new_struct = newsdna->structs[new_struct_index]; + if (old_struct->members_len != new_struct->members_len) { + /* Structs with a different amount of members are not equal. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; + } + if (oldsdna->types_size[old_struct->type] != newsdna->types_size[new_struct->type]) { + /* Structs that don't have the same size are not equal. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; + } + + /* Compare each member individually. */ + for (int member_index = 0; member_index < old_struct->members_len; member_index++) { + SDNA_StructMember *old_member = &old_struct->members[member_index]; + SDNA_StructMember *new_member = &new_struct->members[member_index]; + + const char *old_type_name = oldsdna->types[old_member->type]; + const char *new_type_name = newsdna->types[new_member->type]; + if (!STREQ(old_type_name, new_type_name)) { + /* If two members have a different type in the same place, the structs are not equal. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; + } + + const char *old_member_name = oldsdna->names[old_member->name]; + const char *new_member_name = newsdna->names[new_member->name]; + if (!STREQ(old_member_name, new_member_name)) { + /* If two members have a different name in the same place, the structs are not equal. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; + } + + if (ispointer(old_member_name)) { + if (oldsdna->pointer_size != newsdna->pointer_size) { + /* When the struct contains a pointer, and the pointer sizes differ, the structs are not + * equal. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; + } + } + else { + const int old_member_struct_index = DNA_struct_find_nr(oldsdna, old_type_name); + if (old_member_struct_index >= 0) { + set_compare_flags_for_struct(oldsdna, newsdna, compare_flags, old_member_struct_index); + if (compare_flags[old_member_struct_index] != SDNA_CMP_EQUAL) { + /* If an embedded struct is not equal, the parent struct cannot be equal either. */ + compare_flags[old_struct_index] = SDNA_CMP_NOT_EQUAL; + return; } } } } + + compare_flags[old_struct_index] = SDNA_CMP_EQUAL; } /** * Constructs and returns an array of byte flags with one element for each struct in oldsdna, - * indicating how it compares to newsdna: + * indicating how it compares to newsdna. */ const char *DNA_struct_get_compareflags(const SDNA *oldsdna, const SDNA *newsdna) { @@ -647,130 +702,31 @@ const char *DNA_struct_get_compareflags(const SDNA *oldsdna, const SDNA *newsdna return NULL; } - char *compflags = MEM_callocN(oldsdna->structs_len, "compflags"); - - /* we check all structs in 'oldsdna' and compare them wit @@ Diff output truncated at 10240 characters. @@ _______________________________________________ Bf-blender-cvs mailing list [email protected] https://lists.blender.org/mailman/listinfo/bf-blender-cvs
