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

Tackle performance issues when accessing the preference file #5871

Merged
merged 9 commits into from Feb 4, 2024

Conversation

Garanas
Copy link
Member

@Garanas Garanas commented Feb 3, 2024

The preference file can be accessed with GetPreference, updated with SetPreference and written to the disk with SavePreferences. These functions have some interesting side effects:

--- Retrieves a value in the memory-stored preference file. The value retrieved is a deep copy of what resides in the actual 
--- preference file. Therefore this function can be expensive to use directly - if you're not careful you may be allocating kilobytes worth of data!
---
--- You're encouraged to use `/lua/user/prefs.lua` to interact with the preference file.
---@param string string
---@param default any?
---@return any
function GetPreference(string, default)
end
--- Updates a value in the memory-stored preference file.
---
--- You're encouraged to use `/lua/user/prefs.lua` to interact with the preference file.
---@param key string
---@param obj any
function SetPreference(key, obj)
end
--- Writes the preferences to disk to make it persistent. This is an expensive operation. The 
--- game does this automatically when it exits, there should be no reason to call this manually.
---
--- You're encouraged to use `/lua/user/prefs.lua` to interact with the preference file.
function SavePreferences()
end

Todo:

  • Unbreak the 'all faction templates' from GazUI
WARNING: Error running lua command: c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua(462): bad argument #1 to `gsub' (string expected, got boolean)
         stack traceback:
             [C]: in function `gsub'
             c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua(462): in function `ConvertID'
             c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua(474): in function <c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua:432>
             c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua(779): in function <c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua:771>
             (tail call): ?
             c:\users\4z0t\git\fa\lua\keymap\hotbuild.lua(339): in function `buildAction'
             [string "import("/lua/keymap/hotbuild.lua").buildAct..."](1): in main chunk
             ```

@Garanas Garanas added the area: ui Anything to do with the User Interface of the Game label Feb 3, 2024
@Garanas
Copy link
Member Author

Garanas commented Feb 3, 2024

The old implementation has some interesting consequences. As an example: if a user creates build templates then it can quickly consume your entire preference file. This would be fine if the table would be allocated once and then simply re-used. But that is not the case... in fact, it was re-created all over! When UI code retrieves a template it would call this function:

function GetTemplates()
return Prefs.GetFromCurrentProfile('build_templates')
end

In the old implementation it would then use GetPreference to get the user template data. This would be fine if it was retrieving a constant value. But in reality it is retrieving a table that looks like this:

build_templates = {
    
    -- (...) other templates

    {
        templateData = {
            32,
            32,
            {
                'ueb0102',
                16257,
                0,
                0
            },
            {
                'ueb0102',
                16323,
                8,
                8
            },
            {
                'ueb0102',
                16477,
                16,
                16
            },
            {
                'ueb0102',
                16491,
                24,
                24
            },
            {
                'ueb0102',
                16543,
                16,
                0
            },
            {
                'ueb0102',
                16597,
                24,
                8
            },
            {
                'ueb0102',
                16611,
                0,
                16
            },
            {
                'ueb0102',
                16663,
                8,
                24
            },
            {
                false,
                16680,
                0,
                8
            },
            {
                'ueb1301',
                16835,
                8,
                0
            },
            {
                'ueb1301',
                16975,
                24,
                0
            },
            {
                'ueb1301',
                17156,
                24,
                16
            },
            {
                'ueb1301',
                17322,
                16,
                24
            },
            {
                'ueb1301',
                17517,
                0,
                24
            },
            {
                'ueb4202',
                17878,
                17,
                7
            },
            {
                'ueb1101',
                17997,
                11,
                19
            },
            {
                'ueb1101',
                18033,
                11,
                17
            },
            {
                'ueb1101',
                18044,
                11,
                15
            },
            {
                'ueb1101',
                18051,
                11,
                13
            },
            {
                'ueb1101',
                18074,
                9,
                13
            },
            {
                'ueb1101',
                18099,
                7,
                13
            },
            {
                'ueb1101',
                18205,
                13,
                5
            },
            {
                'ueb1101',
                18241,
                13,
                7
            },
            {
                'ueb1101',
                18253,
                13,
                9
            },
            {
                'ueb1101',
                18290,
                13,
                11
            },
            {
                'ueb1101',
                18306,
                17,
                11
            },
            {
                'ueb1101',
                18313,
                19,
                11
            }
        },
        name = 'Air Factory',
        icon = 'ueb0102'
    },

    -- (...) other templates

},

This is a single entry with just 27 structures in it. It is a little over a kilobyte worth of data. Considering that some people even have entire wall-based paintings in their templates this can easily represent up to dozens, if not hundreds of kilobytes worth of data!

And in the old implementation all of this data would be unique data, recreated each time the user build templates are retrieved! As a few humble examples:

  • (1) When the selection of a user changes

This is a subtle one, because when a unit in the selection dies then the selection changes too. During large battles, this would trigger constantly!

  • (2) When the selection of a user changes and the 'All faction templates' setting is enabled

Note that this is on top of (1). We'd allocate two instances of the user templates table when this option is enabled.

  • (3) When the user is cycling through templates with Hotbuild

Each cycle would recreate the user templates table from scratch!

All of this is bad enough - but funny enough the implementation of (2) actually relies on the templates being unique each time the selection changes, or it crashes. Which is odd - why would you rely on data that may be dozens if not hundreds of kilobytes to be unique each time a selection changes? 😃

Long story short - this is a massive garbage dumb for players that use templates. I barely use templates and the table is already 17 kilobytes:

INFO: GetTemplates
INFO: 17928 bytes

Glad to have found this garbage dumb. It is an amazing candidate to generate stutters as you play the game.

@Garanas
Copy link
Member Author

Garanas commented Feb 3, 2024

The original developers wrote an interface to make it easier to interact with the preference file. You can find it in /lua/user/prefs.lua. One definition is particularly interesting:

fa/lua/user/prefs.lua

Lines 89 to 97 in d0d62d8

function SetToCurrentProfile(fieldName, data)
local profile = GetPreference('profile')
if profile.current then
if profile.profiles[profile.current] then
profile.profiles[profile.current][fieldName] = data
SetPreference('profile', profile)
end
end
end

This function is often used to update the preference file. It makes it easier to update a specific profile. Knowing what we know about GetPreference, what does this function really do?

Yes...

Uhuh!

That is right - for each time we update a field in the preference file via this function we casually deep-copying all the user profiles, including all of their content. Then we update a field. And then we update all the user profiles. For my preference file, this means it is allocating 73 kilobytes for all the profiles, then updates a field and then it effectively throws the table away again. And it would do this for every field we update. And we update fields often 😃 , both while playing (hotkeys, etc) but especially often when we're in the lobby. With the new implementation, the lobby is a lot faster.

The garbage collector is on 🔥 , but we're fine right!

It no longer changes the templates destructively. It will try to
convert the units, and if it fails it simply skips the template.
@Garanas
Copy link
Member Author

Garanas commented Feb 3, 2024

@4z0t / @gordenwunderlich would either if you be available to test the hotbuild alternative implementation for all-faction templates?

And yes - this is not 'do not repeat yourself' proof, but the goal of this PR is not to fix the code style issue. It is to fix a gigantic garbage collection problem.

@Garanas Garanas merged commit 60054b3 into deploy/fafdevelop Feb 4, 2024
3 checks passed
@Garanas Garanas deleted the performance/prefs branch February 4, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ui Anything to do with the User Interface of the Game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant