Remove Map references from MapPreview. #10697
Conversation
pchote
force-pushed
the
map-cleanup-part-2
branch
from
6db6016
to
e8d7853
Compare
|
Confirmed that this reduces the memory usage by ~600MB after loading the RA mod (and generating the map previews) with the resource center maps synced. |
| @@ -26,7 +26,8 @@ public class LobbyLogic : ChromeLogic | |||
| { | |||
| static readonly Action DoNothing = () => { }; | |||
|
|
|||
| public MapPreview Map = MapCache.UnknownMap; | |||
| public MapPreview MapPreview = MapCache.UnknownMap; | |||
| public Map Map = null; | |||
There was a problem hiding this comment.
You're missing null checks in a few places when accessing this field in LobbyMapPreviewLogic.cs and LobbyLogic.cs that cause crashes if the preview is slow to generate. This also isn't assigned in a thread-safe manner from the worker thread.
There was a problem hiding this comment.
Are you sure about this? It shouldn't be any different to what we were already doing - i've just moved the reference from inside the MapPreview to here.
There was a problem hiding this comment.
Yes - you can more easily repro these crashes by simply returning from the worker threads without actually generating the previews.
Here's a few I can see offhand:
LobbyMapPreviewLogic.cs#L28
LobbyMapPreviewLogic.cs#L59
LobbyLogic.cs#L193
LobbyLogic.cs#L320
|
Although the thread-safety worries me in general - if the follow up PR comes along fairly promptly and fixes things by introducing a new parser that doesn't have these issues - then I'm otherwise happy to take this. |
pchote
force-pushed
the
map-cleanup-part-2
branch
from
e8d7853
to
5771e4d
Compare
|
Fixed the LobbyLogic thread safety issues (you can verify this by adding a |
|
Needs a rebase. |
|
👍 |
pchote
force-pushed
the
map-cleanup-part-2
branch
from
5771e4d
to
0db8f23
Compare
|
Rebased. |
pchote
force-pushed
the
map-cleanup-part-2
branch
from
0db8f23
to
8b5592e
Compare
|
Looks good to me. Memory usage for me on the ra main menu with disabled shellmap is down about 20 MB compared to bleed (and about 70/80 MB compared to release). CPU usage compared to release is down by about 10%, btw. Nice. 👍 |
Remove Map references from MapPreview.
|
Added the Changelog entry to the rest of the filesystem stuff. |
Closes #10433.
This removes the ram-hogging Map references from MapPreview. The trade-off is that it takes (significantly) longer to generate the previews in the map chooser. A future PR will reduce this delay by introducing a custom map parsing object that only loads the minimum amount of data required to generate a map preview.
This also removes the visible janks when selecting a map with custom rules in the lobby and missions in the mission browser. It also gets us another step closer to oramod packages.
I have not tested that this actually reduces the memory usage of dedicated servers, but these changes will stand on their own merit even if that does not pan out (which would be very surprising).