Improve Editor Inspector/Theme item lookup performance #93602
Conversation
aaronp64
force-pushed
the
inspector_latency
branch
from
1c22737
to
7fb3982
Compare
scene/theme/theme_db.cpp
Outdated
| p_list->push_back(class_name); | ||
| class_name = ClassDB::get_parent_class_nocheck(class_name); | ||
| } | ||
| r_result.reserve(8); |
There was a problem hiding this comment.
How was this 8 chosen? An arbitrary number may be troublesome
There was a problem hiding this comment.
It's just reserving, it doesn't change anything except worsening performance if the guess is wrong
There was a problem hiding this comment.
In that case wouldn't it be better to reserve memory in the new ClassDB::get_parent_classes_nocheck? I know it sounds odd but ClassDB would "know" better (if someone decided to expand the code). Makes the intent more clear, too.
There was a problem hiding this comment.
8's just an estimate of how many classes might be added, ideally a little higher than needed but it should be fine if it needs to grow past that in some cases. It's mainly to avoid a bunch of small allocations for 1/2/4 items in the vector. It's based on how many are expected to be added by ClassDB, so agreed on moving it there.
Ideally, it would still be reserved before adding p_base_type, which might not be in ClassDB, so could be unexpected to always add it in there. Might shuffle things around a bit to make ClassDB::get_parent_classes_nocheck match up better with ClassDB::is_parent_class (include the passed in class if it's found), and have ThemeDB::get_native_type_dependencies add p_base_type when needed (if ClassDB doesn't find it).
There was a problem hiding this comment.
Made the changes above and renamed get_parent_classes_nocheck to get_inheritance_chain_nocheck, the old name felt confusing since the starting class is now included.
Changes to reduce the latency between changing node selection in the editor and seeing the new node reflected in the Inspector tab - Use LocalVector instead of List for ThemeOwner::get_theme_type_dependencies and related functions - Use LocalVector instead of List for ThemeContext::themes, set_themes(), and get_themes() - Add ClassDB:get_inheritance_chain_nocheck to get all parent/ancestor classes at once, to avoid repeated ClassDB locking overhead - Update BIND_THEME_ITEM macros and ThemeDB::update_class_instance_items to use provided StringNames for call to ThemeItemSetter, instead of creating a new StringName in each call These changes reduce the time taken by EditorInspector::update_tree by around 30-35%
aaronp64
force-pushed
the
inspector_latency
branch
from
7fb3982
to
83f69f2
Compare
Changes to reduce the latency between changing node selection in the editor and seeing the new node reflected in the Inspector tab
LocalVectorinstead ofListforThemeOwner::get_theme_type_dependenciesand related functionsLocalVectorinstead ofListforThemeContext::themes,set_themes(), andget_themes()ClassDB:get_inheritance_chain_nocheckto get all parent/ancestor classes at once, to avoid repeatedClassDBlocking overheadBIND_THEME_ITEMmacros andThemeDB::update_class_instance_itemsto use providedStringNamesfor call toThemeItemSetter, instead of creating a newStringNamein each callThese changes reduce the time taken by
EditorInspector::update_treeby around 30-35%. This should noticeably improve the delays/freezes in #93402, though there's still a lot going on when updating the Inspector. Still could be slow for some nodes, and would depend on CPU.