sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

game.tf2.ext.2.tf2.so introduces significant performance overhead when weapons are created/destroyed

Open KaelaSavia opened this issue 2 years ago • 5 comments

Whenever new tf_weapon_ entity is created, criticals.cpp gives rise to hook related to it. Same goes for unhooking in destruction. https://github.com/alliedmodders/sourcemod/blob/master/extensions/tf2/criticals.cpp#L116 sourcehook

This can result in significant performance issues in majority of custom tf2 gamemodes which make use of TF2_RemoveWeaponSlot or TF2_RemoveAllWeapons or TF2_RegeneratePlayer when weapons are stripped/dropped.

It also occurs with vanilla functionality such as player changing loadout or class.

One solution could be replacing hooking/unhooking by default by TF2_HookCrits(int entity) TF2_UnHookCrits(int entity). But that would break a lot of plugins.

A bit relevant to https://github.com/alliedmodders/sourcemod/issues/1935

KaelaSavia avatar Dec 05 '23 16:12 KaelaSavia

As a bit of update, performance overhead shouldn't happen if none of plugins you have loaded use TF2_CalcIsAttackCritical as criticals.cpp hooking functionality will be not enabled then (if im understanding code correctly).

Until criticals.cpp impact is somewhat mitigated, if you find it impacting your performance i'd personally recommend removing any plugins using it, or replace it's use with detour as dhooks support detours now

KaelaSavia avatar Dec 08 '23 16:12 KaelaSavia

As a blanket statement: Almost all of the hooks on entities providing forwards like this should be vtable hooks.

KyleSanderson avatar Dec 14 '23 01:12 KyleSanderson

We could probably just do a dvp hook on the first of each ent class we see, also storing the ent to a list of ones we care about the hook for, just removing from that when the entity is removed, and all active hooks on no plugins using.

psychonic avatar Dec 14 '23 01:12 psychonic

We could probably just do a dvp hook on the first of each ent class we see, also storing the ent to a list of ones we care about the hook for, just removing from that when the entity is removed, and all active hooks on no plugins using.

That's the correct way (and the way SDKHooks has been), unfortunately there's this whole series: https://github.com/alliedmodders/sourcemod/pull/2094

But yeah, if the vector code wants to be ported here I don't see a problem with it.

KyleSanderson avatar Dec 14 '23 07:12 KyleSanderson

We could probably just do a dvp hook on the first of each ent class we see, also storing the ent to a list of ones we care about the hook for, just removing from that when the entity is removed, and all active hooks on no plugins using.

That's the correct way (and the way SDKHooks has been), unfortunately there's this whole series: https://github.com/alliedmodders/sourcemod/pull/2094

But yeah, if the vector code wants to be ported here I don't see a problem with it.

I'm not very familiar with the SDKHooks code since that change, but I am aware of #2094. I was under the impression that the performance issue was related to hooks being constantly added and removed. Does what I said not address that here? (Only unhooking any of the dvp hooks upon no plugin using the forward, rather than on removal of any one entity, just adding tracking of which entities we care about for when the callback hits?)

psychonic avatar Dec 14 '23 12:12 psychonic