Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes #43156

lekoder · 2020-10-28T10:43:41Z

Godot version:
3.2.3-stable-official, also tested with 3.2.4-beta1

OS/device including version:
Windows 10

Issue description:
Saving a scene in the MRP takes up to couple of minutes, making editing it unusable. Editor is stuck unresponsive at:

Steps to reproduce:

Open MRP
Open res://comms/conversation/MinerConversation.tscn
Save the scene.

Scene should save in fraction of the second, but it hangs on application unresponsive for minutes, making editing the game unpractical. It also happens to every scene this one is instanced in, including "Load as Placeholder" option.

Minimal reproduction project:

slow-saving-mrp.zip

Notes
All the nodes in the scene are instance of same class - it's a representation of dialog tree of the game. I was able to determine that it's somehow related to export var statements - if you remove them from res://comms/ConversationPlayer.gd scene saves with expected speed.

I was not able to produce a synthetic MRP, so this above is stripped-down dialog tree directly from ΔV: Rings of Saturn.

The text was updated successfully, but these errors were encountered:

KoBeWi · 2020-10-28T12:18:34Z

The editor also freezes when you open and close a script from any node.

fire · 2020-10-28T23:34:54Z

I wonder if this is fixed in master?

KoBeWi · 2020-10-28T23:51:19Z

Master requires a separate MRP, because exports are incompatible.

lekoder · 2020-10-29T07:47:26Z

@fire still happening with master / 50da616.

MRP for master: slow-saving-mrp-master.zip

KoBeWi · 2020-10-29T16:09:34Z

I investigated this issue a bit and tracked it down to this method:

godot/core/script_language.cpp

Lines 482 to 524 in 5bb7c7c

    
           void PlaceHolderScriptInstance::update(const List<PropertyInfo> &p_properties, const Map<StringName, Variant> &p_values) { 
        
           	Set<StringName> new_values; 
        
           	for (const List<PropertyInfo>::Element *E = p_properties.front(); E; E = E->next()) { 
        
           		StringName n = E->get().name; 
        
           		new_values.insert(n); 
        
           		if (!values.has(n) || values[n].get_type() != E->get().type) { 
        
           			if (p_values.has(n)) { 
        
           				values[n] = p_values[n]; 
        
           			} 
        
           		} 
        
           	} 
        
           	properties = p_properties; 
        
           	List<StringName> to_remove; 
        
           	for (Map<StringName, Variant>::Element *E = values.front(); E; E = E->next()) { 
        
           		if (!new_values.has(E->key())) { 
        
           			to_remove.push_back(E->key()); 
        
           		} 
        
           		Variant defval; 
        
           		if (script->get_property_default_value(E->key(), defval)) { 
        
           			//remove because it's the same as the default value 
        
           			if (defval == E->get()) { 
        
           				to_remove.push_back(E->key()); 
        
           			} 
        
           		} 
        
           	} 
        
           	while (to_remove.size()) { 
        
           		values.erase(to_remove.front()->get()); 
        
           		to_remove.pop_front(); 
        
           	} 
        
           	if (owner && owner->get_script_instance() == this) { 
        
           		owner->_change_notify(); 
        
           	} 
        
           	//change notify 
        
           	constants.clear(); 
        
           	script->get_constants(&constants); 
        
           }

Placing return at the top will remove the long loading time, not sure why or how to fix it. Could be a good starting point for someone else to give a shot at fixing the issue.

Also here's better MRP for master, because the above one is invalid:
slow-saving-mrp-master.zip

lekoder · 2020-10-29T18:47:24Z

~~I narrowed it further down to this:~~

godot/core/script_language.cpp

Line 518 in 5bb7c7c

owner->_change_notify();

KoBeWi · 2020-10-29T18:54:30Z

Weird, because I checked that and removing it made no difference.

lekoder · 2020-10-29T18:59:07Z

@KoBeWi you are right, I jumped to a conclusion there, sorry.

lekoder · 2020-10-29T22:05:48Z

I narrowed cause of slow saves is here:

godot/editor/editor_node.cpp

Lines 1428 to 1435 in 61c8efe

    
           // force creation of node path cache 
        
           // (hacky but needed for the tree to update properly) 
        
           Node *dummy_scene = sdata->instance(PackedScene::GEN_EDIT_STATE_INSTANCE); 
        
           if (!dummy_scene) { 
        
           	show_accept(TTR("Couldn't save scene. Likely dependencies (instances or inheritance) couldn't be satisfied."), TTR("OK")); 
        
           	return; 
        
           } 
        
           memdelete(dummy_scene);

...which was introduced by 383dea5 (with later changes)

lekoder · 2020-10-30T16:52:18Z

As a workaround I just removed that part from my version of the engine and I can work again, but I suppose it re-introduces the sub-inheritance bug @RandomShaper fixed with that.

lekoder · 2021-04-12T07:52:53Z

Allow me to demonstrate the scale of the problem in 3.3-rc8. This video shows opening and saving scene once, in realtime. It takes 5 minutes to open the scene and over 15 minutes to save it (for each save): https://youtu.be/U-Ut_7x1uWA

KoBeWi · 2021-04-12T13:34:40Z

I wonder what problem was the dummy scene supposed to solve. I only found #7984 and #7976. The former is still valid actually, the latter doesn't have clear reproduction steps.

akien-mga · 2021-04-13T09:41:08Z

#47828 would only fix the slow saving, in my tests it still takes ages to open the scene in the MRP.

Since it might introduce regressions and it's pretty late, I made a version for 3.x which adds a project setting to allow disabling the offending save code: https://github.com/akien-mga/godot/tree/3.x-opt-out-node-paths-update

But if it doesn't help with loading that scene, it seems the problem is deeper than that.

In my tests, disabling that code makes the scene saving take ~40 s (compared to 15 minutes before, so that's a win). Opening the scene still takes ~3 min for me.

Allows working around godotengine#43156. This is an opt-out setting for users who need it, until we can establish that fully removing this code would not reintroduce the bug it aimed to fix (godotengine#7976). Co-authored-by: Mariusz Chwalba <mariusz@chwalba.net>

lekoder · 2021-04-15T11:03:30Z

Not sure if that's any help, but I observe similar slowdowns also when I am renaming objects in tree.

akien-mga · 2021-04-15T11:18:36Z

I think debugging the slow opening for this specific scene should help figure out what the real bug is, and possible solve all symptoms at once (slow opening, slow saving, slow renaming).

It seems to spend a lot of time creating placeholder instances for each instantiated scene and there's plenty of them with multiple levels of sub-instances, which seems to trigger a pathological case:

(gdb) bt
#0  0x0000000001735626 in PropertyInfo::operator= (this=0x12c9b060) at ./core/object.h:144
#1  0x0000000001735775 in List<PropertyInfo, DefaultAllocator>::push_back (this=0x12a00290, value=...) at ./core/list.h:229
#2  0x00000000017f4259 in List<PropertyInfo, DefaultAllocator>::operator= (this=0x12a00290, p_list=...) at ./core/list.h:462
#3  0x00000000042e9179 in PlaceHolderScriptInstance::update (this=0x12a00280, p_properties=..., p_values=...) at core/script_language.cpp:576
#4  0x00000000017ddf86 in GDScript::_update_exports (this=0xc6439e0, r_err=0x0, p_recursive_call=false) at modules/gdscript/gdscript.cpp:512
#5  0x00000000017dcf4a in GDScript::placeholder_instance_create (this=0xc6439e0, p_this=0x12ee6600) at modules/gdscript/gdscript.cpp:331
#6  0x000000000429cdb2 in Object::set_script (this=0x12ee6600, p_script=...) at core/object.cpp:1030
#7  0x0000000004299c2d in Object::set (this=0x12ee6600, p_name=..., p_value=..., r_valid=0x7fffffff83bf) at core/object.cpp:433
#8  0x0000000003c1c5de in SceneState::instance (this=0xc63fd80, p_edit_state=SceneState::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:213
#9  0x0000000003c2740c in PackedScene::instance (this=0x129673b0, p_edit_state=PackedScene::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:1703
#10 0x0000000003c1bba3 in SceneState::instance (this=0x1296b580, p_edit_state=SceneState::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:141
#11 0x0000000003c2740c in PackedScene::instance (this=0x1296c820, p_edit_state=PackedScene::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:1703
#12 0x0000000003c1bba3 in SceneState::instance (this=0x129644f0, p_edit_state=SceneState::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:141
#13 0x0000000003c2740c in PackedScene::instance (this=0x12961450, p_edit_state=PackedScene::GEN_EDIT_STATE_INSTANCE) at scene/resources/packed_scene.cpp:1703
#14 0x0000000003c1bba3 in SceneState::instance (this=0x12975340, p_edit_state=SceneState::GEN_EDIT_STATE_MAIN) at scene/resources/packed_scene.cpp:141
#15 0x0000000003c2740c in PackedScene::instance (this=0xc643e20, p_edit_state=PackedScene::GEN_EDIT_STATE_MAIN) at scene/resources/packed_scene.cpp:1703
#16 0x0000000002a88734 in EditorNode::load_scene (this=0x7f0dda0, p_scene=..., p_ignore_broken_deps=false, p_set_inherited=false, p_clear_errors=true, p_force_open_imported=false, p_silent_change_tab=false)
    at editor/editor_node.cpp:3623
#17 0x0000000002a88f5f in EditorNode::open_request (this=0x7f0dda0, p_path=...) at editor/editor_node.cpp:3685
<cut rest of the backtrace, not relevant>

Won't have time to debug further for now, anyone is welcome to pick it up from here.

Allows working around godotengine#43156. This is an opt-out setting for users who need it, until we can establish that fully removing this code would not reintroduce the bug it aimed to fix (godotengine#7976). Co-authored-by: Mariusz Chwalba <mariusz@chwalba.net>

johnfn · 2021-05-26T22:40:08Z

I'd like to chime in to say that this is an issue for me as well, and removing the dummy scene does in fact make saving the scene become basically instantaneous.

lekoder changed the title ~~Saving scene with exports takes up to minutes~~ Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes Oct 28, 2020

akien-mga added bug topic:editor labels Oct 28, 2020

akien-mga added this to the 4.0 milestone Oct 28, 2020

akien-mga assigned KoBeWi and RandomShaper Apr 12, 2021

KoBeWi mentioned this issue Apr 12, 2021

Removed dummy scene during saving #47828

Closed

RandomShaper mentioned this issue Jun 13, 2021

Fix slow load/save of scenes with many instances of the same script #49570

Merged

akien-mga closed this as completed in #49570 Jun 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes #43156

Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes #43156

lekoder commented Oct 28, 2020

KoBeWi commented Oct 28, 2020

fire commented Oct 28, 2020

KoBeWi commented Oct 28, 2020 •

edited

Loading

lekoder commented Oct 29, 2020

KoBeWi commented Oct 29, 2020

lekoder commented Oct 29, 2020 •

edited

Loading

KoBeWi commented Oct 29, 2020

lekoder commented Oct 29, 2020

lekoder commented Oct 29, 2020

lekoder commented Oct 30, 2020

lekoder commented Apr 12, 2021

KoBeWi commented Apr 12, 2021

akien-mga commented Apr 13, 2021 •

edited

Loading

lekoder commented Apr 15, 2021

akien-mga commented Apr 15, 2021

johnfn commented May 26, 2021

Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes #43156

Saving complex scene with multiple exports var statements and instanced sub-scenes takes up to minutes #43156

Comments

lekoder commented Oct 28, 2020

KoBeWi commented Oct 28, 2020

fire commented Oct 28, 2020

KoBeWi commented Oct 28, 2020 • edited Loading

lekoder commented Oct 29, 2020

KoBeWi commented Oct 29, 2020

lekoder commented Oct 29, 2020 • edited Loading

KoBeWi commented Oct 29, 2020

lekoder commented Oct 29, 2020

lekoder commented Oct 29, 2020

lekoder commented Oct 30, 2020

lekoder commented Apr 12, 2021

KoBeWi commented Apr 12, 2021

akien-mga commented Apr 13, 2021 • edited Loading

lekoder commented Apr 15, 2021

akien-mga commented Apr 15, 2021

johnfn commented May 26, 2021

KoBeWi commented Oct 28, 2020 •

edited

Loading

lekoder commented Oct 29, 2020 •

edited

Loading

akien-mga commented Apr 13, 2021 •

edited

Loading