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

Cache result of gmod.GetGamemode() in hook.Run #2075

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mgetJane
Copy link
Contributor

@mgetJane mgetJane commented Apr 6, 2024

hook.Run currently calls gmod.GetGamemode() which is a C call so it's unnecessarily slower than it should be

the function's return value is always equivalent to GAMEMODE or GM

lua_run local s=SysTime() for i=1,1000000 do hook.Run("test") end print(SysTime()-s)
> local s=SysTime() for i=1,1000000 do hook.Run("test") end print(SysTime()-s)...
0.106619362

lua_run function gmod.GetGamemode() return GAMEMODE end
> function gmod.GetGamemode() return GAMEMODE end...

lua_run local s=SysTime() for i=1,1000000 do hook.Run("test") end print(SysTime()-s)
> local s=SysTime() for i=1,1000000 do hook.Run("test") end print(SysTime()-s)...
0.00039914499999583

lua_run local s=SysTime() for i=1,1000000 do hook.Call("test", GAMEMODE) end print(SysTime()-s)
> local s=SysTime() for i=1,1000000 do hook.Call("test", GAMEMODE) end print(SysTime()-s)...
0.00066306600000132

@mgetJane
Copy link
Contributor Author

mgetJane commented Apr 6, 2024

also, what does gmod.GetGamemode() even do exactly? why does it need to be a C function?

an alternative to this PR is simply overriding that function with a lua version like so:

function gmod.GetGamemode()
	return GAMEMODE or GM
end

@robotboy655 robotboy655 added the Enhancement The pull request enhances current functionality. label Apr 19, 2024
@robotboy655
Copy link
Collaborator

I imagine this is its only advantage
image

@mgetJane
Copy link
Contributor Author

mgetJane commented Apr 20, 2024

I imagine this is its only advantage [image]

yeah, code_gs mentioned in the discord that gmod.GetGamemode() ensures you can still get the gamemode table even if the value of GAMEMODE gets changed

imo i would say it's still preferable to have hook.Run use GAMEMODE for the performance boost, since you probably have far bigger problems if some addon decides to set GAMEMODE to a different value

@Astralcircle
Copy link
Contributor

If the method with GAMEMODE is not accepted, then you can do something like this:

local gm_cache

function Run( name, ... )
    if not gm_cache then
        gm_cache = gmod and gmod.GetGamemode() or nil
    end

    return Call( name, gm_cache, ... )
end

@mgetJane mgetJane changed the title Don't call gmod.GetGamemode() in hook.Run Cache result of gmod.GetGamemode() in hook.Run Nov 1, 2024
@mgetJane
Copy link
Contributor Author

mgetJane commented Nov 1, 2024

If the method with GAMEMODE is not accepted, then you can do something like this:

local gm_cache

function Run( name, ... )
    if not gm_cache then
        gm_cache = gmod and gmod.GetGamemode() or nil
    end

    return Call( name, gm_cache, ... )
end

thanks, seems like the best compromise

still not sure why the GAMEMODE variable getting changed is a concern, since that'd completely break everything else anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement The pull request enhances current functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants