search

inkle / ink-unity-integration

Unity integration for the open source ink narrative scripting language.
http://www.inklestudios.com/ink
Other
578 stars 101 forks source link

Performance issue in Project view with large projects #182

Closed shinymerlyn closed 1 year ago

shinymerlyn commented 1 year ago

The function InkLibrary.IsInkFile causes a large slowdown on projects with a lot of assets, when typing in the filter field of the Project view. This is because it is creating a closure and doing a foreach on all current ink files for every object in the project (or at least every object that passes the filter/gets drawn). https://github.com/inkle/ink-unity-integration/blob/9b75c439b3213e714be74adf9300208f1860ac5c/Packages/Ink/Editor/Core/InkEditorUtils.cs#L279

I have done a local hack to put the paths into a HashSet<string> and do the appropriate maintenance on this set when the inkLibrary list is modified. For me this improved the performance significantly. This dictionary can probably be behind a #if UNITY_EDITOR flag.

My hack got me much better performance, though maybe there's further gains to be had, i.e. somehow completely avoiding this call during the EditorApplication.projectWindowItemOnGUI callback. Someone with better knowledge of the project and why this call even is done can probably do a better job on a PR than I have time for.

Repro:

  1. Build a somewhat large game (ours has 12k+) and integrate this plugin.
  2. Type text into the Project filter field (e.g. t:Prefab, etc).

Expected: Editor is roughly as fast as when this plugin isn't installed

Actual: Editor is much slower. Hundreds of milliseconds elapse after each character is typed in

tomkail commented 1 year ago

Thank you for this report! I'll try to get to this when I find time, sounds like it needs a fix rather urgently! I've tagged as Help Wanted in case anyone else in the community has time. If you could share your changes that'd be useful too, especially if you've tested lots and can confirm it works.

tomkail commented 1 year ago

I've improved performance on this by first checking if the file is a folder. I like your fix and suggestions better but I'm hesitant to add more caches - it's such a common cause of bugs! Ideally we wouldn't need that part of the function at all though, it's only to support files without extensions which we do for legacy reasons. Beyond that ideally we wouldn't need the InkBrowserIcons class at all; I'd very much like to find a better way to draw icons in the editor. I had a quick play with EditorGUIUtility.SetIconForObject but it kept crashing Unity. Additionally it only allows icons to be drawn with a single size - the current approach allows for different icons at different sizes, which I find makes them much nicer. I'm gonna close this out and make a new task to replace InkBrowserIcons.