Fix GameStateSnapshots not being freed

Updated GameStateSnapshot to inherit from RefCounted, to be automatically deleted when reference count reaches zero, and removed GameStateSnapshotRef wrapper class.
This commit is contained in:
aaronp64 2025-11-30 12:48:09 -05:00
parent 7ed0b61676
commit ed6181c943
4 changed files with 25 additions and 49 deletions

View file

@ -41,7 +41,9 @@
#include "core/os/time.h" #include "core/os/time.h"
#include "editor/debugger/editor_debugger_node.h" #include "editor/debugger/editor_debugger_node.h"
#include "editor/debugger/script_editor_debugger.h" #include "editor/debugger/script_editor_debugger.h"
#include "editor/docks/inspector_dock.h"
#include "editor/editor_node.h" #include "editor/editor_node.h"
#include "editor/inspector/editor_inspector.h"
#include "editor/themes/editor_scale.h" #include "editor/themes/editor_scale.h"
#include "scene/gui/button.h" #include "scene/gui/button.h"
#include "scene/gui/label.h" #include "scene/gui/label.h"
@ -167,7 +169,7 @@ TreeItem *ObjectDBProfilerPanel::_add_snapshot_button(const String &p_snapshot_f
void ObjectDBProfilerPanel::_show_selected_snapshot() { void ObjectDBProfilerPanel::_show_selected_snapshot() {
if (snapshot_list->get_selected()->get_text(0) == (String)diff_button->get_selected_metadata()) { if (snapshot_list->get_selected()->get_text(0) == (String)diff_button->get_selected_metadata()) {
for (int i = 0; i < diff_button->get_item_count(); i++) { for (int i = 0; i < diff_button->get_item_count(); i++) {
if (diff_button->get_item_text(i) == current_snapshot->get_snapshot()->name) { if (diff_button->get_item_text(i) == current_snapshot->name) {
diff_button->select(i); diff_button->select(i);
break; break;
} }
@ -184,7 +186,7 @@ void ObjectDBProfilerPanel::_on_snapshot_deselected() {
_update_enabled_diff_items(); _update_enabled_diff_items();
} }
Ref<GameStateSnapshotRef> ObjectDBProfilerPanel::get_snapshot(const String &p_snapshot_file_name) { Ref<GameStateSnapshot> ObjectDBProfilerPanel::get_snapshot(const String &p_snapshot_file_name) {
if (snapshot_cache.has(p_snapshot_file_name)) { if (snapshot_cache.has(p_snapshot_file_name)) {
return snapshot_cache.get(p_snapshot_file_name); return snapshot_cache.get(p_snapshot_file_name);
} }
@ -201,7 +203,7 @@ Ref<GameStateSnapshotRef> ObjectDBProfilerPanel::get_snapshot(const String &p_sn
Vector<uint8_t> content = snapshot_file->get_buffer(snapshot_file->get_length()); // We want to split on newlines, so normalize them. Vector<uint8_t> content = snapshot_file->get_buffer(snapshot_file->get_length()); // We want to split on newlines, so normalize them.
ERR_FAIL_COND_V_MSG(content.is_empty(), nullptr, "ObjectDB Snapshot file is empty: " + full_file_path); ERR_FAIL_COND_V_MSG(content.is_empty(), nullptr, "ObjectDB Snapshot file is empty: " + full_file_path);
Ref<GameStateSnapshotRef> snapshot = GameStateSnapshot::create_ref(p_snapshot_file_name, content); Ref<GameStateSnapshot> snapshot = GameStateSnapshot::create_ref(p_snapshot_file_name, content);
if (snapshot.is_valid()) { if (snapshot.is_valid()) {
snapshot_cache.insert(p_snapshot_file_name, snapshot); snapshot_cache.insert(p_snapshot_file_name, snapshot);
} }
@ -225,8 +227,8 @@ void ObjectDBProfilerPanel::_view_tab_changed(int p_tab_idx) {
// Populating tabs only on tab changed because we're handling a lot of data, // Populating tabs only on tab changed because we're handling a lot of data,
// and the editor freezes for a while if we try to populate every tab at once. // and the editor freezes for a while if we try to populate every tab at once.
SnapshotView *view = cast_to<SnapshotView>(view_tabs->get_current_tab_control()); SnapshotView *view = cast_to<SnapshotView>(view_tabs->get_current_tab_control());
GameStateSnapshot *snapshot = current_snapshot.is_null() ? nullptr : current_snapshot->get_snapshot(); GameStateSnapshot *snapshot = current_snapshot.ptr();
GameStateSnapshot *diff = diff_snapshot.is_null() ? nullptr : diff_snapshot->get_snapshot(); GameStateSnapshot *diff = diff_snapshot.ptr();
if (snapshot != nullptr && !view->is_showing_snapshot(snapshot, diff)) { if (snapshot != nullptr && !view->is_showing_snapshot(snapshot, diff)) {
view->show_snapshot(snapshot, diff); view->show_snapshot(snapshot, diff);
} }
@ -237,6 +239,11 @@ void ObjectDBProfilerPanel::clear_snapshot(bool p_update_view_tabs) {
view->clear_snapshot(); view->clear_snapshot();
} }
const Object *edited_object = InspectorDock::get_inspector_singleton()->get_edited_object();
if (Object::cast_to<SnapshotDataObject>(edited_object)) {
EditorNode::get_singleton()->push_item(nullptr);
}
current_snapshot.unref(); current_snapshot.unref();
diff_snapshot.unref(); diff_snapshot.unref();
@ -333,7 +340,7 @@ void ObjectDBProfilerPanel::_edit_snapshot_name() {
ObjectDBProfilerPanel::ObjectDBProfilerPanel() { ObjectDBProfilerPanel::ObjectDBProfilerPanel() {
set_name(TTRC("ObjectDB Profiler")); set_name(TTRC("ObjectDB Profiler"));
snapshot_cache = LRUCache<String, Ref<GameStateSnapshotRef>>(SNAPSHOT_CACHE_MAX_SIZE); snapshot_cache = LRUCache<String, Ref<GameStateSnapshot>>(SNAPSHOT_CACHE_MAX_SIZE);
EditorDebuggerNode::get_singleton()->get_current_debugger()->connect("breaked", callable_mp(this, &ObjectDBProfilerPanel::_on_debug_breaked)); EditorDebuggerNode::get_singleton()->get_current_debugger()->connect("breaked", callable_mp(this, &ObjectDBProfilerPanel::_on_debug_breaked));

View file

@ -69,9 +69,9 @@ protected:
HashMap<int, PartialSnapshot> partial_snapshots; HashMap<int, PartialSnapshot> partial_snapshots;
LocalVector<SnapshotView *> views; LocalVector<SnapshotView *> views;
Ref<GameStateSnapshotRef> current_snapshot; Ref<GameStateSnapshot> current_snapshot;
Ref<GameStateSnapshotRef> diff_snapshot; Ref<GameStateSnapshot> diff_snapshot;
LRUCache<String, Ref<GameStateSnapshotRef>> snapshot_cache; LRUCache<String, Ref<GameStateSnapshot>> snapshot_cache;
void _request_object_snapshot(); void _request_object_snapshot();
void _begin_object_snapshot(); void _begin_object_snapshot();
@ -95,7 +95,7 @@ public:
void receive_snapshot(int p_request_id); void receive_snapshot(int p_request_id);
void show_snapshot(const String &p_snapshot_file_name, const String &p_snapshot_diff_file_name); void show_snapshot(const String &p_snapshot_file_name, const String &p_snapshot_diff_file_name);
void clear_snapshot(bool p_update_view_tabs = true); void clear_snapshot(bool p_update_view_tabs = true);
Ref<GameStateSnapshotRef> get_snapshot(const String &p_snapshot_file_name); Ref<GameStateSnapshot> get_snapshot(const String &p_snapshot_file_name);
void set_enabled(bool p_enabled); void set_enabled(bool p_enabled);
void add_view(SnapshotView *p_to_add); void add_view(SnapshotView *p_to_add);

View file

@ -318,11 +318,9 @@ void GameStateSnapshot::recompute_references() {
} }
} }
Ref<GameStateSnapshotRef> GameStateSnapshot::create_ref(const String &p_snapshot_name, const Vector<uint8_t> &p_snapshot_buffer) { Ref<GameStateSnapshot> GameStateSnapshot::create_ref(const String &p_snapshot_name, const Vector<uint8_t> &p_snapshot_buffer) {
// A ref to a refcounted object which is a wrapper of a non-refcounted object. Ref<GameStateSnapshot> snapshot;
Ref<GameStateSnapshotRef> sn; snapshot.instantiate();
sn.instantiate(memnew(GameStateSnapshot));
GameStateSnapshot *snapshot = sn->get_snapshot();
snapshot->name = p_snapshot_name; snapshot->name = p_snapshot_name;
// Snapshots may have been created by an older version of the editor. Handle parsing old snapshot versions here based on the version number. // Snapshots may have been created by an older version of the editor. Handle parsing old snapshot versions here based on the version number.
@ -347,13 +345,13 @@ Ref<GameStateSnapshotRef> GameStateSnapshot::create_ref(const String &p_snapshot
continue; continue;
} }
snapshot->objects[obj.id] = memnew(SnapshotDataObject(obj, snapshot, resource_cache)); snapshot->objects[obj.id] = memnew(SnapshotDataObject(obj, snapshot.ptr(), resource_cache));
snapshot->objects[obj.id]->extra_debug_data = (Dictionary)snapshot_data[i + 3]; snapshot->objects[obj.id]->extra_debug_data = (Dictionary)snapshot_data[i + 3];
} }
snapshot->recompute_references(); snapshot->recompute_references();
print_verbose("Resource cache hits: " + String::num(resource_cache.hits) + ". Resource cache misses: " + String::num(resource_cache.misses)); print_verbose("Resource cache hits: " + String::num(resource_cache.hits) + ". Resource cache misses: " + String::num(resource_cache.misses));
return sn; return snapshot;
} }
GameStateSnapshot::~GameStateSnapshot() { GameStateSnapshot::~GameStateSnapshot() {
@ -361,15 +359,3 @@ GameStateSnapshot::~GameStateSnapshot() {
memdelete(item.value); memdelete(item.value);
} }
} }
bool GameStateSnapshotRef::unreference() {
bool die = RefCounted::unreference();
if (die) {
memdelete(gamestate_snapshot);
}
return die;
}
GameStateSnapshot *GameStateSnapshotRef::get_snapshot() {
return gamestate_snapshot;
}

View file

@ -33,7 +33,6 @@
#include "editor/debugger/editor_debugger_inspector.h" #include "editor/debugger/editor_debugger_inspector.h"
class GameStateSnapshot; class GameStateSnapshot;
class GameStateSnapshotRef;
class SnapshotDataObject : public Object { class SnapshotDataObject : public Object {
GDCLASS(SnapshotDataObject, Object); GDCLASS(SnapshotDataObject, Object);
@ -77,8 +76,8 @@ protected:
static void _bind_methods(); static void _bind_methods();
}; };
class GameStateSnapshot : public Object { class GameStateSnapshot : public RefCounted {
GDCLASS(GameStateSnapshot, Object); GDCLASS(GameStateSnapshot, RefCounted);
void _get_outbound_references(Variant &p_var, HashMap<String, ObjectID> &r_ret_val, const String &p_current_path = ""); void _get_outbound_references(Variant &p_var, HashMap<String, ObjectID> &r_ret_val, const String &p_current_path = "");
void _get_rc_cycles(SnapshotDataObject *p_obj, SnapshotDataObject *p_source_obj, HashSet<SnapshotDataObject *> p_traversed_objs, LocalVector<String> &r_ret_val, const String &p_current_path = ""); void _get_rc_cycles(SnapshotDataObject *p_obj, SnapshotDataObject *p_source_obj, HashSet<SnapshotDataObject *> p_traversed_objs, LocalVector<String> &r_ret_val, const String &p_current_path = "");
@ -88,24 +87,8 @@ public:
HashMap<ObjectID, SnapshotDataObject *> objects; HashMap<ObjectID, SnapshotDataObject *> objects;
Dictionary snapshot_context; Dictionary snapshot_context;
// Ideally, this would extend EditorDebuggerRemoteObject and be refcounted, but we can't have it both ways. static Ref<GameStateSnapshot> create_ref(const String &p_snapshot_name, const Vector<uint8_t> &p_snapshot_buffer);
// So, instead we have this static 'constructor' that returns a RefCounted wrapper around a GameStateSnapshot.
static Ref<GameStateSnapshotRef> create_ref(const String &p_snapshot_name, const Vector<uint8_t> &p_snapshot_buffer);
~GameStateSnapshot(); ~GameStateSnapshot();
void recompute_references(); void recompute_references();
}; };
// Thin RefCounted wrapper around a GameStateSnapshot.
class GameStateSnapshotRef : public RefCounted {
GDCLASS(GameStateSnapshotRef, RefCounted);
GameStateSnapshot *gamestate_snapshot = nullptr;
public:
GameStateSnapshotRef(GameStateSnapshot *p_gss) :
gamestate_snapshot(p_gss) {}
bool unreference();
GameStateSnapshot *get_snapshot();
};