SHsuperCM / CITResewn

Fabric implementation of mcpatcher's cit
MIT License
146 stars 72 forks source link

avoid parsing nbt strings if they do not smell like JSON #343

Closed wchristian closed 3 months ago

wchristian commented 8 months ago

repro steps:

  1. install hypixel+ resource pack
  2. install Roughly Enough Resources
  3. set cache behavior to "every tick" †
  4. connect to mc.hypixel.net
  5. hit f3+2
  6. open inventory
  7. browse to the later pages in REI with the top right button

† yes, that is not optimal for performance, but even with a bigger value there, big spikes occur

inspired by #315, combines well with #320

it's a regex, but in many cases it only looks at the first 1-2 characters and aborts VERY quickly, while other types of non-regex checks would need to parse the whole string and eat massive amounts of time

of course the most correct solution is to have a cache that can avoid needing to parse at all, but also distributes its invalidations over time to avoid spikes, however the current performance impacts are ridiculous and deserve quick fixes

SHsuperCM commented 3 months ago

it's a regex, but in many cases it only looks at the first 1-2 characters and aborts VERY quickly, while other types of non-regex checks would need to parse the whole string and eat massive amounts of time

Could you explain this part? for what reason would it need to parse the whole string with a non-regex solution? You only need to run up to the first non empty character which is very simple and should abort even faster than the regex on non-jsons

SHsuperCM commented 3 months ago

After testing I found that a pure java check is significantly faster than the regex counterpart.

SHsuperCM commented 3 months ago

Superseded by upstream (1427fb8)