GC perf issue on load

[flint acorn]

OK, i've been nosing through the code in a decompiler to see about the json zip extraction, but I noticed something inefficient relating to the garbage collector. Seeing as that's a hot topic wrt stuttering I thought I'd mention it.

In DataHandler.JsontoData routine, every json file is inspected and every key entry is read and added to a dict. However, every entry is also added to a string (not a stringbuilder) in a loop to create a huge string. This string is then discarded 😀

OK, there is a special case test right at the end that will print this string to log, but it would probably be better if the test was done first, and the string appending only performed if true.

The string is also used in the exception handler but only to show the last entry read, but as every valid key is added to the dict in the loop, changing this to print out the dict's entries would achieve the same result.

I'm sure the constant reallocation of memory for this string for every one of the many json files, with something like half a gb of data between them is killing the GC. Not to mention a small perf hit during load.

[hallow flame]

This is some good observation!
I've been a bit hesitant to touch loading code yet as that's not my area and since it's onetime it's less of a factor to the periodic stuttering that most people are actually frustrated with.
After we get some of the other stuff taken care of, I'll definitely take a look back at loading and use this thread as a starting point! Thanks~ 🫡

[flint acorn]

Take a look at that 1 function, it's self-contained so you won't break anything by altering the debug logging to be better. Take a note of the zip load bug report as I added some code change suggestions there for you to consider, for when you're ready.

[flint acorn]

Just a bump as you updated the zip loading. Did you deal with this massive allocation fest? It could easily be part of the periodic stutter as this mountain of old string allocs is not going to be cleared by the GC quickly.

[radiant raven]

Lemme add this to our issue tracker to take a look. Swapping out that string alloc for a StringBuilder seems pretty save, and yeah, might also reduce some excessive mem allocs.

Good find!