Repository navigation
Add an ObjectDB Profiling Tool - #97210
Conversation
There was a problem hiding this comment.
Tested locally, it works as expected on Windows 11 23H2. This is a great start 🙂
Some feedback:
- I'd make the snapshot store the exact Godot version (including commit hash) that was used to generate it, so that you can compare it in diffs. Use something like this to generate the stored version string:
godot/editor/project_manager.cpp
Lines 1463 to 1467 in c3e16cd
- The Snapshot Overview line shouldn't have an underline, as people can mistake it for a clickable link. Just use a bold font to indicate it's a heading.
-
I noticed timestamp files are named according to the UTC time, not local time. This is inconsistent with log file naming which always uses local time.
-
It might be a good idea to compress ObjectDB snapshot files as they can get quite large in projects with lots of objects. If I put a 3.2 MB snapshot in a ZIP, it's only 224 KB once compressed, which is a more than 10x compression ratio. FileAccess has compression methods available for this, along with the Compression singleton.
-
Whenever I save a snapshot, I see this in the terminal output (not the editor Output panel):
Starting to snapshot
WARNING: Property not found: ProjectSettings
at: ProjectSettings::_get (core\config\project_settings.cpp:351)
snapshot length: 7089
The unknown setting warning should be fixed, and I suggest moving the snapshot-related prints to print_verbose() so that they only appear in verbose mode.
- I got a sporadic crash when clicking the Filter objects field in the ObjectDB debugger. I haven't been able to reproduce it again though:
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.4.dev.custom_build (036846c0284c4176ace06ca38d466bfb167618a1)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] StringName::~StringName (C:\Users\Hugo\Documents\Git\godotengine\godot\core\string\string_name.h:187)
[1] StringName::~StringName (C:\Users\Hugo\Documents\Git\godotengine\godot\core\string\string_name.h:187)
[2] EditorStringNames::~EditorStringNames
[3] EditorStringNames::`scalar deleting destructor'
[4] memdelete<EditorStringNames> (C:\Users\Hugo\Documents\Git\godotengine\godot\core\os\memory.h:140)
[5] EditorStringNames::free (C:\Users\Hugo\Documents\Git\godotengine\godot\editor\editor_string_names.h:45)
[6] unregister_editor_types (C:\Users\Hugo\Documents\Git\godotengine\godot\editor\register_editor_types.cpp:310)
[7] Main::cleanup (C:\Users\Hugo\Documents\Git\godotengine\godot\main\main.cpp:4522)
[8] widechar_main (C:\Users\Hugo\Documents\Git\godotengine\godot\platform\windows\godot_windows.cpp:184)
[9] _main (C:\Users\Hugo\Documents\Git\godotengine\godot\platform\windows\godot_windows.cpp:206)
[10] main (C:\Users\Hugo\Documents\Git\godotengine\godot\platform\windows\godot_windows.cpp:220)
[11] WinMain (C:\Users\Hugo\Documents\Git\godotengine\godot\platform\windows\godot_windows.cpp:234)
[12] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[13] <couldn't map PC to fn name>
-- END OF BACKTRACE --
- At the minimum bottom panel height, it's possible for the Objects panel to overlap the bottom of the editor:
- The Diff against dropdown should gray out the currently selected snapshot, as there's no point in comparing a snapshot against itself:
When selecting the snapshot that is currently being diffed against, the two should be swapped together (i.e. Diff against automatically changes).
- The JSON view should use the editor's code font, as this kind of data displays best with fixed-width fonts. See #89690 for an example of this, which also adds line numbers and syntax highlighting (there's a built-in JSON syntax highlighter available in the editor).
What does "RC" mean here? I'm guessing it means "reference count". I think we have the space to spell it out here:
- I wonder if tooltips can be added to each column in the Objects and RefCounted sections to explain some of the columns, especially In/Out in Objects and Total Refs/User Refs/User Cycles which are not obvious at a glance.
|
A couple of things to keep in mind:
|
|
I think this is ready for another review. Some notes on the changes I've made: Fixed:
Not Fixed/Need Guidance:
Generally speaking, taking a snaphot means calling every single getter in a game. If any of them are buggy or do something surprising, the snapshot might fail or log errors like you saw in |
|
The latest build error I fixed had a similar stack trace to the sporadic crash @Calinou reported above. Not sure if it's the same root cause, but this commit may have fixed the filter box crash described above. |
There was a problem hiding this comment.
Tested locally, it works as expected. Keep up the good work 🙂
I've noticed a few remaining UI issues:
- The line spacing in the bullet point list at the right of the RefCounted tab should be increased for more comfortable reading:
This issue isn't present in bullet point lists found elsewhere, such as the Summary tab.
- The JSON tab doesn't use a fixed-width font correctly, it uses the default project font instead:
Check where you are getting the editor's code font; it may be done too early and should be done in NOTIFICATION_THEME_CHANGED instead.
-
Loading the JSON tab can take a long time (easily 10+ seconds on a high-end machine when comparing two snapshots on Truck Town), so I suggest displaying some kind of "Generating JSON, this may take a while..." text in the TextEdit(s) then actually putting the JSON text in place one or two frames later (
call_deferred()can help here). -
In a future PR, we could add a keyboard shortcut that can be pressed in the running project to capture an ObjectDB snapshot (similar to how pressing F8 quits the game when the project window is focused). This would make it easier to capture snapshots during gameplay.
cb6a64a to
80f8bb3
Compare
I've gone ahead and added this now, so there should be another tree item separate from the scene root called "Orphan Nodes" with the I wasn't sure whether to add it at the top or bottom. I figured keeping the scene root at the top made the most sense, but I'd be happy to change it around. |
80f8bb3 to
c2f1399
Compare
c2f1399 to
461063c
Compare
461063c to
3528e83
Compare
|
I went ahead and squashed all of my commits to a single one. We still need to squash the rest of them, but I figured that can wait until the merge, in case the original PR commit is of some use until then. |
|
I tested this PR on my game, and it allowed me to find every memory leaks in my game in less than 1 hour, so to me on a user point of view that's a big yes. It's very efficient and well done. I also want to mention, that this is a very important feature for Godot to have. As far as I'm concerned stable release should implement it asap. |
|
Hello! It's been a long time since I last looked at this PR, and I figured I would check in. Is there anything I can do to help move it forwards? I don't think I was very helpful with the smaller cleanup work, but if there's any bigger units of work that have come up, I'd be happy to tackle them. |
|
The PR is already queued for 4.6, so you don't need to do anything. |
Maybe we can keep this PR to just two commits: one for the Tree column title tooltip text and another for ObjectDB profiling. |
| add_child(classes_view); | ||
| classes_view->set_anchors_preset(LayoutPreset::PRESET_FULL_RECT); | ||
| classes_view->set_v_size_flags(SizeFlags::SIZE_EXPAND_FILL); | ||
| classes_view->set_h_size_flags(SizeFlags::SIZE_EXPAND_FILL); |
There was a problem hiding this comment.
Setting size flags are not needed for children of Control nodes. both can be removed.
| classes_view->set_split_offset(0); | ||
|
|
||
| VBoxContainer *class_list_column = memnew(VBoxContainer); | ||
| class_list_column->set_v_size_flags(SizeFlags::SIZE_EXPAND_FILL); |
There was a problem hiding this comment.
Setting vertical size flags on HSplitContainer is not needed, the default fill is enough.
| class_tree->set_column_expand(3, false); | ||
| } | ||
| class_tree->connect(SceneStringName(item_selected), callable_mp(this, &SnapshotClassView::_class_selected)); | ||
| class_tree->set_h_size_flags(SizeFlags::SIZE_EXPAND_FILL); |
There was a problem hiding this comment.
Setting horizontal size flags for VBoxContainer children is not needed.
| class_tree->connect(SceneStringName(item_selected), callable_mp(this, &SnapshotClassView::_class_selected)); | ||
| class_tree->set_h_size_flags(SizeFlags::SIZE_EXPAND_FILL); | ||
| class_tree->set_v_size_flags(SizeFlags::SIZE_EXPAND_FILL); | ||
| class_tree->set_anchors_preset(LayoutPreset::PRESET_FULL_RECT); |
There was a problem hiding this comment.
Setting anchors preset is not needed for Container children.
| classes_view->add_child(object_lists); | ||
| object_lists->set_custom_minimum_size(Size2(150 * EDSCALE, 0)); | ||
| object_lists->set_h_size_flags(SizeFlags::SIZE_EXPAND_FILL); | ||
| object_lists->set_v_size_flags(SizeFlags::SIZE_EXPAND_FILL); |
There was a problem hiding this comment.
This line can be removed.
| explainer_lines->add_child(l3); | ||
|
|
||
| ScrollContainer *sc = memnew(ScrollContainer); | ||
| sc->set_anchors_preset(LayoutPreset::PRESET_FULL_RECT); |
There was a problem hiding this comment.
This line can be removed.
|
|
||
| VBoxContainer *content = memnew(VBoxContainer); | ||
| mc->add_child(content); | ||
| content->set_anchors_preset(LayoutPreset::PRESET_FULL_RECT); |
There was a problem hiding this comment.
This line can be removed.
| Ref<StyleBoxFlat> sbf; | ||
| sbf.instantiate(); | ||
| sbf->set_bg_color(EditorNode::get_singleton()->get_editor_theme()->get_color("dark_color_3", "Editor")); | ||
| pc->add_theme_style_override("panel", sbf); |
There was a problem hiding this comment.
| pc->add_theme_style_override("panel", sbf); | |
| pc->add_theme_style_override("panel", EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("title_button_normal"), SNAME("Tree"))); |
sbf can be removed.
| sbf->set_bg_color(EditorNode::get_singleton()->get_editor_theme()->get_color("dark_color_3", "Editor")); | ||
| pc->add_theme_style_override("panel", sbf); | ||
| content->add_child(pc); | ||
| pc->set_anchors_preset(LayoutPreset::PRESET_TOP_WIDE); |
There was a problem hiding this comment.
This line can be removed.
|
|
||
| explainer_text = memnew(CenterContainer); | ||
| explainer_text->set_v_size_flags(SizeFlags::SIZE_EXPAND_FILL); | ||
| explainer_text->set_h_size_flags(SizeFlags::SIZE_EXPAND_FILL); |
There was a problem hiding this comment.
This line can be removed.
|
Note: All theme changes should be updated inside |
Repiteo
left a comment
There was a problem hiding this comment.
@WhalesState I'd encourage setting up a separate PR with those suggestions, as integrating this tool early in 4.6 (i.e. now) takes priority
|
Thanks! Congratulations on your first merged contribution! 🎉 |
…ot-debugger Add an ObjectDB Profiling Tool
…ot-debugger Add an ObjectDB Profiling Tool
…ot-debugger Add an ObjectDB Profiling Tool







Intro
This PR adds a new tab to the debug menu that can help profile a game's memory usage. It's a follow up to this proposal - godotengine/godot-proposals#10628.
Specifically, this lets you save a snapshot of all the objects in a running game's ObjectDB to disk. It then lets you view the snapshot and diff two snapshots against each other. This is meant to work similarly to Chrome's heap shopshot tool or unity's memory profiler.
UI Overview
The column on the left is universal functionality.
user://objectdb_snapshotsfolder. Click a snapshot to view it. Right click a snapshot to rename, delete, or view in your file browser.The space on the right contains 6 tabs. Each tab shows the contents of the snapshot in a different way. All tabs support diffing in some form.
a. Total memory used
b. Name and creation time of the snapshot
c. Total object count
d. Total node count
Other thoughts
Other than a few fixes in core, most of the code is in a new module. The biggest change to core is adding a lock to
Object::setso that objects cannot be modified whenObjectDB::debug_objectsis running. I'm fairly new to C++, so I'm hoping someone can double check I'm using locks correctly there.Snapshots can be in the 10-100s of megabytes, and the debug network peer has a maximum buffer size of 8mb, so I'm chunking the snapshots into smaller slices and sending them 6mb at a time.
Since snapshots can be pretty big, I store them in an LRUCache and only keep 10 in the editor's memory at a time. If it feels like selecting a snapshot for the first time is slow, that's probably why.
Also along those lines, I don't populate each tab until you actually click on it. It makes loading a snapshot a little faster, but slows down clicking on each tab. The Objects, RefCounted, and JSON tabs can be brutally slow.
In order to show snapshot objects in the inspector, I'm extending the
EditorDebuggerRemoteObjectclass used in the scene tree's remote tab.Snapshots have the
.odb_snapshotfile extension. The first line is a version number, and the second like is the output of callingMarshalls::variant_to_base64. I was going to save this as a .tres, but realized 99% of the data I was saving wasn't a resource, andMarshalls::variant_to_base64was the easiest way I could find to get the data in and out of a string.