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

Very long scripts tend to slow writing down #74733

Closed
Daniel-da-Silva opened this issue Mar 10, 2023 · 8 comments
Closed

Very long scripts tend to slow writing down #74733

Daniel-da-Silva opened this issue Mar 10, 2023 · 8 comments

Comments

@Daniel-da-Silva
Copy link

Daniel-da-Silva commented Mar 10, 2023

Godot version

3.5.2 and previous versions

System information

MX Linux, SSD, i5-6500

Issue description

Hey,

I tend to work on large scripts. When a script gets very long, 10k+ lines scrolling can be laggy (if you use the scrollbar and drag it) and writing new stuff can take considerable amount of time, the inputs are taken but until it shows on screen it can take a second (just write something very fast, especially at the end of the script). It does not happen always but in more than 50% of tries.

I could use an external editor but I would really like to avoid using one.

Edit: Sorry for Closing and Reopening that was not my intention. I wanted to add: I just tested it on Godot 4.0 - the problem is not there, not when using the Forward nor the Compatibility renderer if that helps.

Steps to reproduce

Work with a large script

Minimal reproduction project

Minimal_Project.zip

I don't know how to not include the .godot folder but couldn't find one with ls -a. This is a project with 40k lines to test it properly.

Just write something very fast, especially at the end of the script.

@lawnjelly
Copy link
Member

lawnjelly commented Mar 11, 2023

This is a debug profile but it should give some indication:

script_editor_profile

Looks like a large amount of time searching in color_region_cache which is a Map.

And this is a release profile, which is more about pressing keys at the bottom of the script:

script_editor_profile_release

Understandably a lot of time is spent in parsing, perhaps it is parsing from the top of the file. I don't know whether this is occurring on the main thread and causing a long pause. And again the line syntax highlighting is taking a long time.

Update

The line caching in TextEdit::_is_line_in_region() doesn't appear to be working correctly, it seems to be searching through every previous line for every line in the script, in order to find the syntax highlighting region. 🤔

@lawnjelly
Copy link
Member

OK with the bug fix above, the syntax highlighting seems no longer a problem, now the editor is fast in the MRP except for parsing, which causes a pause. Here is the new revised profile with the bug fix:

script_editor_profile3

I can immediately see the parsing is using an index operator with a linked list, which is a massive no-no.. 😱

@lawnjelly
Copy link
Member

Ok with #74782 we are getting there. Also in the parser there's a fair amount of string compares, which should probably be using hash tables.

Ideally it would be nice to just not parse the entire script, but that might be more hassle than the benefits. Also the parsing would typically be done on a separate thread, I'm not sure why it's stalling the main thread. 🤔

@vnen
Copy link
Member

vnen commented Mar 12, 2023

Ideally it would be nice to just not parse the entire script, but that might be more hassle than the benefits.

This would probably be difficult to implement. Just figuring out what region to parse could lead to issues.

Also the parsing would typically be done on a separate thread, I'm not sure why it's stalling the main thread.

The current system does not lend itself well for this. It needs the parser info to provide things like error messages and code completion.

I actually have some ideas for this, like caching the parse tree, which would help moving it to a thread as it could have a clear sync point. It would also be possible to cancel the current parsing if another needs to start. This could also help with selective parsing but I'm still not sure if it would be worth it.

Though I don't know when this will be done and I doubt it would be possible to backport to 3.x, at least not with simple cherry-picking.

@lawnjelly
Copy link
Member

The current system does not lend itself well for this. It needs the parser info to provide things like error messages and code completion.

Yeah it could be a barrel of worms for a corner case, and with these fixes it should be significantly better for large files anyway.

@akien-mga akien-mga modified the milestones: 3.x, 3.6 Mar 12, 2023
@lawnjelly
Copy link
Member

I've just tested with the 3 PRs merged in 3.x, and it's LOADS faster. It's now fine for editing with such a large file, so I think this issue can be closed.

We should be able to cherry pick the PRs to the next 3.5 as well, I suspect.

@Daniel-da-Silva
Copy link
Author

@lawnjelly Thank you very much for your work and effort!

@oeleo1
Copy link

oeleo1 commented Apr 14, 2023

Many thanks here from me as well! We used to struggle with an 8K line long stript where typing became extra slow (on modern machines) with all the automatic parsing and error checking. Now working on this script is a bliss. Well done and much appreciated @lawnjelly !

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

5 participants