MehVahdJukaar / Moonlight

GNU General Public License v3.0
53 stars 29 forks source link

[Performance] Terrible performance for listing resources #244

Closed SettingDust closed 3 months ago

SettingDust commented 3 months ago

https://github.com/MehVahdJukaar/Moonlight/blob/1.20/common/src/main/java/net/mehvahdjukaar/moonlight/api/resources/pack/DynamicResourcePack.java#L215-L217

The filter is slow. Should add a multimap for namespace to identifiers. Iterate the map and get the bytes of id then

I tried to add one with my own mod with mixin. The startup time reduced from 220 to 120 secs.

Another small question https://github.com/MehVahdJukaar/Moonlight/blob/1.20/common/src/main/java/net/mehvahdjukaar/moonlight/api/resources/pack/DynamicResourcePack.java#L304-L310. Why not just clear the resources instead of iterate?

MehVahdJukaar commented 3 months ago

with which mods did you profile this? given the dynamic pack can be miniscule or huge depending on what you have installed? As for the second probably leftover from when i had conditional removal

MehVahdJukaar commented 3 months ago

I did some testing loading 10 thousand dummy resources and while your method was definitely an improvement, the old one took 0.2 seconds to complete across all .listResources call in a single reload.

Its definitely an improvement so ill add but i dont underswant where you crazily higher load time comes from

SettingDust commented 3 months ago

There are 20k+ resources in the moonlight pack. My pack has 500+ mods on fabric.

Below is a JFR of the launching, taking 230 seconds. client-2024-08-18-084526.zip

As you can see, it's took 124 seconds. 图片

A launch taking 130 seconds with my optimization client-2024-08-18-090245.zip It's taking 1 second 图片

MehVahdJukaar commented 3 months ago

Jesus 500 mods . Well I tested with half as many resources and got nowhere near that slowdown, was still a fraction of a second even with the old method. Something else is definitely at play here as that method should not take 100seconds to just iterate offer some elements. That's crazy long

SettingDust commented 3 months ago

There is JFR anyway. I don't know why. Disable all the resource packs isn't helpful.

MehVahdJukaar commented 3 months ago

Maybe the act itself of profiling is what causes such s slowdown. Tbh idk, that call seems to be taking 1000 more time than it is for me despite both having tested it with the same order or magnitude of resources in the pack

SettingDust commented 3 months ago

Maybe the act itself of profiling is what causes such s slowdown. Tbh idk, that call seems to be taking 1000 more time than it is for me despite both having tested it with the same order or magnitude of resources in the pack

I write the profiling mod to find out the reason XD

MehVahdJukaar commented 3 months ago

I have added a search trie for maximum search performance.srill I could keep cutting the time I half each time but that won't stop it to take thousand times more than it is for me.theres definitely some other issue going on here, maybe some mod abusing that call

SettingDust commented 3 months ago

The JFR should show that. I guess it's polytone XD 图片

MehVahdJukaar commented 3 months ago

So poltrone adds around 20 of those list resources calls for each of their folders. Other mods you have might add some more . Obviously the more you have of those the slower it gets but the time each take is still very disproportionate to what I could test. Even assuming you have around 50 of these reloaders that call list resources it doesn't come near the 1000 times increase that I saw so definitely something else is at play. Anyways feel free to test the new version with the search trie i added. It would expect it to be some couple order of magnotudde faster for how many resources you have there. Still if it takes anything more than a fraction of a second it still an indicator that something else is slowing down this as it should be in the milliseconds

SettingDust commented 3 months ago

I tried the latest before. It's working well

图片 It's called 13k times.

6K from polytone, 6k from continuity. But moremcmeta taking extra calls to continuity calls. So, continuity+moremcmeta 6k