Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor startup and FileSystem dock slowdown from favorites #89646

Closed
permelin opened this issue Mar 18, 2024 · 6 comments
Closed

Editor startup and FileSystem dock slowdown from favorites #89646

permelin opened this issue Mar 18, 2024 · 6 comments

Comments

@permelin
Copy link
Contributor

permelin commented Mar 18, 2024

Tested versions

The problem is only present in 5cf38f8 (#77932 merge) plus the bugfix in #89642 (not yet merged)

System information

Godot v4.3.dev (264a7677e) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2080 (nvidia) - Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz (8 Threads)

Issue description

With one or more tscn files marked as favorite in the FileSystem dock, my editor startup time increases by a lot (as a function of the size of the favorited scenes), and every time I right-click a file, the editor becomes unresponsive for at least one second.

Startup time with zero, one and two large scenes as favorites respectively: 13, 28, 53 seconds

Tracing what's going on, apparently #77932 loads the scene file in search of a possible custom icon. And because the editor startup process in general is extremely inefficient, this is done 4-6 times for every favorite. And for some reason it's also done for all favorites on every right-click on any file.

cc: @KoBeWi (again, sorry)

Steps to reproduce

  1. Mark any large scene file as favorite.
  2. Start the editor and see how long it takes for FileSystem to show and the editor to become responsive.

Nothing useful is shown with --benchmark unfortunately so you need to time it yourself.

You'll need the fix in #89642 because without it nothing is loaded as intended.

Minimal reproduction project (MRP)

Not provided because you need a pretty large scene to really get the effect.

@permelin permelin changed the title Editor startup and FileSystem dock slowdown Editor startup and FileSystem dock slowdown from favorites Mar 18, 2024
@JekSun97
Copy link
Contributor

Starting from version 4.0, my editor has basically become twice as slow as in 3.x

@AThousandShips
Copy link
Member

AThousandShips commented Mar 18, 2024

You'll need the fix in #89642 because without it nothing is loaded as intended.

Are you encountering this bug at all without this PR? If not make sure it's not actually caused by it, or exposed by it in a way that needs to be resolved in it

@permelin
Copy link
Contributor Author

Are you encountering this bug at all without this PR? If not make sure it's not actually caused by it, or exposed by it in a way that needs to be resolved in it

Without the fix in #89642 there is no slowdown. But the fix is not causing the slowdown. #77932 tries to do something that is really slow, but it just happens to shoot itself in the foot trying to do it. The fix removes the foot-shooting, which reveals the bigger issue.

@permelin
Copy link
Contributor Author

@KoBeWi was this maybe what you actually intended to do?

--- a/editor/filesystem_dock.cpp
+++ b/editor/filesystem_dock.cpp
@@ -453,7 +453,7 @@ void FileSystemDock::_update_tree(const Vector<String> &p_uncollapsed_paths, boo
 			int index;
 			EditorFileSystemDirectory *dir = EditorFileSystem::get_singleton()->find_file(favorite, &index);
 			if (dir) {
-				icon = _get_tree_item_icon(dir->get_file_import_is_valid(index), dir->get_file_type(index), dir->get_file_path(index));
+				icon = _get_tree_item_icon(dir->get_file_import_is_valid(index), dir->get_file_type(index), _get_entry_script_icon(dir, index));
 			} else {
 				icon = get_editor_theme_icon(SNAME("File"));
 			}

@AThousandShips
Copy link
Member

AThousandShips commented Mar 18, 2024

Then it is caused by that PR, it introduces a behavior that is slow, in a place where it shouldn't be used, so it should be fixed in that PR :) Granted the original PR didn't use the correct paths, but the error isn't in some third place, which would be the case if it wasn't caused by this change

@permelin
Copy link
Contributor Author

I've located the second bug that caused the slowdown when the first was fixed. I've amended #89642 with that one too and I'm closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants