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

Load&save in separate thread #37

Open
FINDarkside opened this issue Nov 11, 2019 · 8 comments
Open

Load&save in separate thread #37

FINDarkside opened this issue Nov 11, 2019 · 8 comments
Labels
enhancement New feature or request

Comments

@FINDarkside
Copy link
Owner

Loading big saves causes the UI to hang for a while, so loading the save should probably be done in separate thread. At least save button and save selection dropdown needs to be disabled when save/load is happening.

@Pt-Djefferson
Copy link
Contributor

Almost all UI elements shell to be disabled in case of load or save except tabs labels. And some progress bar can be added at status bar.

@FINDarkside
Copy link
Owner Author

It'd be ideal to not disable controls when saving though. But because the bottleneck shouldn't actually be writing the save to the disk it'd be quite problematic as obviously no changes after clicking save shoud apply. Doing deep clone of the save data which could be supplied to the other thread wouldn't be trivial either, as the extraFields dictionary in DynamicSerializable uses object instance as key.

There's probably lot of stuff to be optimized in DynamicSerializable anyway, at least lot of the reflection stuff could be cached. Haven't done any profiling to check if that's even one of the bottlenecks though.

@FINDarkside
Copy link
Owner Author

Took a look at performance profile, most of the time is spent in JObject.Parse and JToken.ToObject. If performance really was a big problem writing a custom JsonTextReader would probably speed things up.

@Pt-Djefferson
Copy link
Contributor

Or we can get (and set) only necessary data from text by precompiled regexp and by not all data at a time but partialy (depends on focused tab). It will be not nice as it is now but fast.

@FINDarkside
Copy link
Owner Author

FINDarkside commented Nov 12, 2019

Managed to reduce the load time by ~30% with couple simple changes. JObject.Parse remains as the bottleneck when loading saves though. It doesn't really help that the save file is a hot mess of nested serialized json strings and json strings compressed stored as byte array which is then again converted back to json 😄

Or we can get (and set) only necessary data from text by precompiled regexp and by not all data at a time but partialy (depends on focused tab). It will be not nice as it is now but fast.

I'm not sure if you understood you correctly, but editing json with regex is pretty problematic as json isn't a regular language and therefore it cannot be fully parsed with regex. I don't think splitting the work by tab would actually bring any noticeable difference, I'd guess that the biggest problem is the first deserialization&serialization as it can be 5MB of byte arrays. No matter what we do, we need to turn big byte arrays to json.

Just profiled save, and JSON.net is the bottleneck there as well, so at least it's not the DynamicSerializable that would be adding significant slowdown. I'll check if there's some easy optimizations though.

@FINDarkside
Copy link
Owner Author

Reduced save time by over 60% (in my example save) with d7a8b61. It would probably be worth to do the same thing with all primitive arrays and lists, but byte arrays were obviously the biggest problem.

@FINDarkside
Copy link
Owner Author

FINDarkside commented Nov 12, 2019

Reduced save time by further ~25% by caching results of GetCustomAttribute. eef9c27

At the moment most of the save time is spent on JsonTextWriter.WriteValue, CLZF.lzf_compress and JsonConvert.SerializeObject so there probably aren't any big (trivial) gains anymore.

@FINDarkside
Copy link
Owner Author

Caching Type.GetFields and Type.GetProperties doesn't seem to give any noticeable improvements.

@FINDarkside FINDarkside added the enhancement New feature or request label Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants