Nak2 / StormFox

StormFox for garrysmod
Apache License 2.0
29 stars 13 forks source link

cvars.AddChangeCallback spam performance issues #21

Open xaviergmail opened 5 years ago

xaviergmail commented 5 years ago

fprofiler

I'm currently working on something else and I don't have the time to make a PR to improve this, but it could definitely be worth having a look at.

SupinePandora43 commented 5 years ago

@xaviergmail what is on screenshot?! (which utility)

xaviergmail commented 5 years ago

That's FProfiler @SupinePandora43

On Sat, Jul 6, 2019, 11:44 PM SupinePandora43 notifications@github.com wrote:

@xaviergmail https://github.com/xaviergmail what is on screenshot?! (which utility)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Nak2/StormFox/issues/21?email_source=notifications&email_token=AAD7OMVE32CCOG2CTZPHEPDP6FYD5A5CNFSM4H6UHXGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZLEQOA#issuecomment-508971064, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD7OMR5EWVSRXBI27RPPTTP6FYD5ANCNFSM4H6UHXGA .

xaviergmail commented 5 years ago

I've moved the cvars.AddChangeCallback calls over to the AddConvar helper function and the difference is night and day. This SetNetworkData function seems like it's doing a lot of heavy lifting for no reason, and does so in a render hook.

MuckerMayhem commented 5 years ago

What was the change you made exactly? I'm looking to fix this locally for our server to tide us over until an official fix can be pushed, if you have an example I can probably work of of that.

xaviergmail commented 5 years ago

@MuckerMayhem

Remove the cvars.AddChangeCallback function call from from lua/stormfox/framework/sh_network.lua inside StormFox.SetNetworkData and move it to

lua/stormfor/lib/sh_convars.lua

    local function AddConVar(str,default,helptext)
        StormFox.convars[str] = true
        if ConVarExists(str) then return end
        CreateConVar(str,default, { FCVAR_REPLICATED, FCVAR_ARCHIVE }, helptext)
        cvars.AddChangeCallback(str, function( convar_name, value_old, value_new )
            StormFox.SetNetworkData("con_" .. convar_name, value_new)
        end,"SF_Netupdate-" .. str )
    end

Sorry I can't provide a proper diff right now. Also I don't know if this has any unwanted side effects, but I don't seem to see any right now.

MuckerMayhem commented 5 years ago

Muck appreciated, I think I've got it in right.