FormularSumo / Star-Wars-Galaxy-Collection

A remake of the discontinued Star Wars Force Collection game https://formularsumo.github.io/Star-Wars-Galaxy-Collection-Web/ https://play.google.com/store/apps/details?id=com.formularsumo.starwarsforcecollectionremake.embed
GNU Affero General Public License v3.0
6 stars 0 forks source link

Investigate moving file saving/serialisation off the main thread #83

Closed FormularSumo closed 3 months ago

FormularSumo commented 3 months ago

Following on from https://github.com/FormularSumo/Star-Wars-Galaxy-Collection/issues/81, file saving can also have an impact on load times in the deck editor, especially when sorting the deck. While file loading/deserialising needs to be done on the main thread really as its a precursor to loading other things, file saving/serialisation can be moved off. It shouldn't really matter if this is delayed slightly as it still only takes a fraction of a second or so. The only issue I can see is if the program tries to read the a file while it's still being saved. If needed it could check if the file writing thread is running before saving. Or ideally I could potentially rewrite/ensure that deckeditstate only loads from file the first time it's loaded rather than anytime during execution, which should be fine as the the P1cards exists this whole time so can be used instead. Given the time it takes to to exit and enter deckeditstate it shouldn't be possible then to cause this race condition but might be worth checking on the first P1deck file load that it's not in use.

FormularSumo commented 3 months ago

Time spent on main thread loading deckeditstate:

dumpLoveFile (pretty much all bitser serialising) - 0.078 sort cards - 0.078 createBackground - 0.067 loading character images (prior to being moved offthread) - 0.0013 create P1cards/remove duplicates from deck - 0.00013 load player deck file - 0.000048

FormularSumo commented 3 months ago

Improvements testing on my laptop now vs start of today (average of 4 runs)

Loading deckeditor - 56 --> 58 FPS (half as much FPS drop) Time taken to load deckeditor - 0.0706 --> 0.0532 seconds (24.6% reduction) Sorting deck editor - 50 --> 55 FPS (also half as much)

FormularSumo commented 3 months ago

File saving is only done now upon swapping cards (only deck cards if in sandbox) and when auto-sorting/emptying your deck (when not in sandbox).

When sandbox is off, time spent on auto-sorting: Total time - 0.0298 dumpLoveFile (only for cards, didn't realise deck was separate) - 0.00169

Sandbox on: Total time - 0.0300 Sorting characters - 0.0274 Creating characters - 0.0015

Based on this, I don't think it is worth moving file saving onto a different thread at the moment as the performance impact it currently has on these actions is very minimal. The main culprit for using the auto/blank button is sorting. And for swapping cards there isn't really any performance impact (might be worth testing this on weaker hardware/web version though).

FormularSumo commented 3 months ago

Upon further investigation, it seems like while it's sorting that's causing the main performance issue when sorting, it's not the actual sorting but ironically the file saving that's incurred when calling P1deckedit, as this resaves the file every time a card is edited. This could be moved off the main thread, however I think before doing that it's worth batching these saves together.

FormularSumo commented 3 months ago

Having now done this: Total time - 0.0092 Sorting time - 0.0050 File saving time - 0.00273

So now file saving time is only taking around 30% of the total time. There might still be some benefit to moving it off-thread for slower hardware but for most devices the whole operation now takes less than a frame so should not have a noticeable performance impact.

FormularSumo commented 3 months ago

Sorting in the deck editor on my laptop after batching file saves: 55 FPS -- 60 FPS (>5x improvement)

The the total improvement to sorting has now gone from 50 FPS to 60 FPS, or >10x improvement

FormularSumo commented 3 months ago

Considering that there is no frame drops/lag when performing carrying out file saving (when swapping cards or sorting), and that the total time to do this is less than a frame on most systems, I think I'm going to conclude that it's not worth moving file writing offthread as there wouldn't be any noticeable improvement to doing it.

Moving it offthread would also mean that the current imageDecoder thread would need rewriting to support file writing (perhaps it could take an argument as to which task to perform), or that more threads would need to be created, both of which would add quite a lot of complexity to the current threading system.