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

Reparenting nodes is extremely slow in certain situations #84910

Closed
KoBeWi opened this issue Nov 14, 2023 · 3 comments · Fixed by #85502
Closed

Reparenting nodes is extremely slow in certain situations #84910

KoBeWi opened this issue Nov 14, 2023 · 3 comments · Fixed by #85502

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Nov 14, 2023

Godot version

4.2 beta6

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 30.0.15.1403) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

godot.windows.editor.dev.x86_64_3sKvkuw0Am.mp4

The culprit is one of the recursive check PRs. I profiled the editor and in the case above most of the time is spent in get_property_list() method of TileAtlasSource, called by _check_node_path_recursive() in SceneTreeDock. Apparently this tile method is very slow and here it's called tens of times.

Not sure what the fix would be, maybe the editor should skip external resources (is it safe to assume they don't point to anything on the scene?). Or we could have a list of exceptions that are known to be slow and don't have NodePaths.

Steps to reproduce

  1. Open the MRP
  2. Move Node2D under TileMap

Minimal reproduction project

LagSet.zip

@atirut-w
Copy link
Contributor

Reverting #80721 (might be "one of the recursive check PRs") using RC2 used as base commit fixes this problem and, for some reason, does not seem to reintroduce the bugs fixed in the PR. I have no earthly idea how this works, and it definitely have to be tested by some other people.

@atirut-w
Copy link
Contributor

Would it be alright for 4.2 to depend on this issue getting fixed? I can imagine this being a huge productivity issue for any decently-sized project.

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 29, 2023

does not seem to reintroduce the bugs fixed in the PR

Reverting it does reintroduce #74155 and similar issues.
I think limiting it to built-in resources only is an acceptable compromise.

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

Successfully merging a pull request may close this issue.

4 participants