C++ TFS 1.2 Game::checkCreatureAttack high usage in performance

[henkas]

Hello,
noticed a huge lag when like 15-20 people uses spells or overall melee attacks in a same place, ping jumps even to a 1k and after checking dispatcher this is the log

[21/11/2023 21:47:25]
Thread: 1 Cpu usage: 94.9291% Idle: 1.94614% Other: 3.12471% Players online: 165
 Time (ms)     Calls     Rel usage %    Real usage % Description
     13408   2001327       47.08344%       44.69591% std::bind(&Game::checkCreatureAttack, &g_game, getID())
      5736      1133       20.14303%       19.12160% std::bind(&Game::checkCreatures, this, (index + 1) % EVENT_CREATURECOUNT)
      3938    126800       13.82834%       13.12713% std::bind(&Game::checkCreatureWalk, &g_game, getID())
      1912      1458        6.71657%        6.37598% functor
      1002     14507        3.52165%        3.34307% &Game::playerSay
       600       115        2.10871%        2.00178% std::bind(&Game::checkDecay, this)
       409      2913        1.43680%        1.36394% std::bind(&Spawn::scheduleSpawn, this, spawnId, sb, interval - 1400)
       397      1381        1.39481%        1.32409% &Game::parsePlayerExtendedOpcode
       274      7699        0.96220%        0.91341% std::bind(&ProtocolGame::parseNewWalk, getThis(), playerWalkId, predictiveWalkId, playerId, playerPosition, flags, path)
       245      3212        0.86185%        0.81814% std::bind(&LuaEnvironment::executeTimerEvent, &g_luaEnvironment, lastTimerEventId)
       230       779        0.80871%        0.76770% std::bind(&Game::executeDeath, &g_game, getID())
       115      2276        0.40509%        0.38455% &Game::playerUseItem
        33      4100        0.11704%        0.11111% &Game::playerTurn
        32         1        0.11461%        0.10880% std::bind(&ProtocolGame::logout, getThis(), true, false)

I would say this is quite a large resource usage in CPU isnt it? How i know its spells or melee related? Because when it stoped attacking and casting spells on a creature lag disappeared completetly

this is all code parts that could be related

    if (creature == attackedCreature || (creature == this && attackedCreature)) {
        if (newPos.z != oldPos.z || !canSee(attackedCreature->getPosition())) {
            onCreatureDisappear(attackedCreature, false);
        } else {
            if (hasExtraSwing()) {
                //our target is moving lets see if we can get in hit
                g_dispatcher.addTask(createTask(std::bind(&Game::checkCreatureAttack, &g_game, getID())));
            }

            if (newTile->getZone() != oldTile->getZone()) {
                onAttackedCreatureChangeZone(attackedCreature->getZone());
            }
        }
    }
}

void Game::checkCreatureAttack(uint32_t creatureId)
{
    Creature* creature = getCreatureByID(creatureId);
    if (creature && creature->getHealth() > 0) {
        creature->onAttacking(0);
    }
}

bool Monster::selectTarget(Creature* creature)
{
    if (!isTarget(creature)) {
        return false;
    }

    auto it = std::find(targetList.begin(), targetList.end(), creature);
    if (it == targetList.end()) {
        //Target not found in our target list.
        return false;
    }

    Player* player = creature->getPlayer();
    if (player)
        if (!trySaga(player))
            return false;

    if (isHostile() || isSummon()) {
        if (setAttackedCreature(creature) && !isSummon()) {
            g_dispatcher.addTask(createTask(std::bind(&Game::checkCreatureAttack, &g_game, getID())));
        }
    }
    return setFollowCreature(creature);
}

bool Player::setAttackedCreature(Creature* creature)
{
    if (creature)
    {
        if (Player* p = creature->getPlayer())
        {
            if (isPartner(p))
            {
                sendCancelTarget();
                return false;
            }
        }
    }

    if (!Creature::setAttackedCreature(creature)) {
        sendCancelTarget();
        return false;
    }

    if (chaseMode == CHASEMODE_FOLLOW && creature) {
        if (followCreature != creature) {
            //chase opponent
            setFollowCreature(creature);
        }
    }
    else if (followCreature) {
        setFollowCreature(nullptr);
    }

    if (creature) {
        g_dispatcher.addTask(createTask(std::bind(&Game::checkCreatureAttack, &g_game, getID())));
    }
    return true;
}

void Player::doAttacking(uint32_t)
{
    if (lastAttack == 0) {
        lastAttack = OTSYS_TIME() - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((OTSYS_TIME() - lastAttack) >= getAttackSpeed()) {
        bool result = false;

        Item* tool = getWeapon();
        const Weapon* weapon = g_weapons->getWeapon(tool);
        uint32_t delay = getAttackSpeed();

        if (weapon) {
            if (!weapon->interruptSwing()) {
                result = weapon->useWeapon(this, tool, attackedCreature);
            }
            else if (!canDoAction()) {
                delay = getNextActionTime();
            }
            else {
                result = weapon->useWeapon(this, tool, attackedCreature);
            }
        }
        else {
            result = Weapon::useFist(this, attackedCreature);
        }

        /* There's not even the need to check if player is dual wielding: the variable is ignored otherwise. */
        switchAttackHand();

        SchedulerTask* task = createSchedulerTask(std::max<uint32_t>(SCHEDULER_MINTICKS, delay), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
        g_scheduler.addEvent(task);

        if (result) {
            lastAttack = OTSYS_TIME();
        }
    }
}

 

[henkas]

bump

 

[Sarah Wesker]

If the attack speed on your server is very low, it is evident that 15 or 20+ players will cause this method to be called many times.
When you choose a target, the method is called: doAttacking
then from doAttacking checkCreatureAttack is called and checkCreatureAttack calls doAttacking again

It is a calling loop that never stops, and the speed at which this happens is the attack speed

In each iteration other methods are called and some more things are done. such as sending effects, checking distances, checking line of sight, sending combat, calculating damage, calling lua events, sending text to spectators, scanning the map for spectators, creating temporary objects, etc...

So if your attack speed is 100 milliseconds, everything I mentioned above will be repeated every 100 milliseconds for each player who is attacking.
That's a lot of stress for a single processor core.

-----------------------------------------------------------------------------------------------------------------------------------------------

What you can maybe do is start by checking your combat lua scripts, maybe there are some unoptimized scripts that slow down this process
onHealthChange, onManaChange, onTargetCombat, onAreaCombat, ect...
Maybe modifications to your sources regarding combat?

 

[marcelomnzs]

*off: sorry, I don't know how to help with this problem,
but could you tell me how to get a detailed bug report like this, please?

 

[Error 502]

*off: sorry, I don't know how to help with this problem,
but could you tell me how to get a detailed bug report like this, please?

that's from .cpp

OTS Statistics by kondra ([email protected]) by gesior · Pull Request #4306 · otland/forgottenserver

I've got permission from kondra (otclient@************) to release this code. He only requested to leave information about author kondra (otclient@************) in commit message. It's well known '...

github.com

I don't think it reports lua in this way

--example
[29/01/2023 16:08:37]
Thread: 1 Cpu usage: 0.628848% Idle: 99.6671% Other: -0.295933% Players online: 0
 Time (ms)     Calls     Rel usage %    Real usage % Description
      1        1               1               1     scripts/custom/target_custom.lua:onTargetCombat

would be interesting if were like that xd

 

[henkas]

If the attack speed on your server is very low, it is evident that 15 or 20+ players will cause this method to be called many times.
When you choose a target, the method is called: doAttacking
then from doAttacking checkCreatureAttack is called and checkCreatureAttack calls doAttacking again

It is a calling loop that never stops, and the speed at which this happens is the attack speed

In each iteration other methods are called and some more things are done. such as sending effects, checking distances, checking line of sight, sending combat, calculating damage, calling lua events, sending text to spectators, scanning the map for spectators, creating temporary objects, etc...

So if your attack speed is 100 milliseconds, everything I mentioned above will be repeated every 100 milliseconds for each player who is attacking.
That's a lot of stress for a single processor core.

-----------------------------------------------------------------------------------------------------------------------------------------------

What you can maybe do is start by checking your combat lua scripts, maybe there are some unoptimized scripts that slow down this process
onHealthChange, onManaChange, onTargetCombat, onAreaCombat, ect...
Maybe modifications to your sources regarding combat?

Cant find any sus onHealthChange code. Attack speed on my game is pretty high, for sure higher than original tibia. For source i do have attack speed speed, the higher skill_club is the faster you hit, and i have critical hit, dual wielding and probably from source side that is related to combat is thats it

 

[Sarah Wesker]

Cant find any sus onHealthChange code. Attack speed on my game is pretty high, for sure higher than original tibia. For source i do have attack speed speed, the higher skill_club is the faster you hit, and i have critical hit, dual wielding and probably from source side that is related to combat is thats it

I have checked the time each call takes in this attack cycle
doAttacking consumes about 0.2 milliseconds when the attack is applied
and 99% of this small period of time is taken up by Lua onHealthChange events

Here is the event loop that consumes all the time:

forgottenserver/src/game.cpp at 2946e9e4c4991d79ac64b2300e52cb70b876cda9 · otland/forgottenserver

A free and open-source MMORPG server emulator written in C++ - otland/forgottenserver

github.com

It seems like a short time, but if you multiply this time by 100 players and with an exaggerated attack speed of 100 milliseconds, we can calculate that each second the processor would be occupied for at least 220 milliseconds.
This is equivalent to approximately 20% and we are assuming that we only have 1 onHealthChange event and with some simple IF
Now imagine when you have many more events and many more systems and bad unoptimized scripts.

I did these tests with an old processor that runs at a maximum of 3.60 GHz, so these times could improve if your computer/VPS is of good quality

Consider increasing the CPU capacity of your machine and you may have better results.
But remember that the main problem is allowing players to attack like machine gun fire.

Here is a similar topic, if you want to follow it: C++ - High CPU Usage classicAttackSpeed (https://otland.net/threads/high-cpu-usage-classicattackspeed.285956/#post-2731083)

 

[henkas]

I have checked the time each call takes in this attack cycle
doAttacking consumes about 0.2 milliseconds when the attack is applied
and 99% of this small period of time is taken up by Lua onHealthChange events

Here is the event loop that consumes all the time:

forgottenserver/src/game.cpp at 2946e9e4c4991d79ac64b2300e52cb70b876cda9 · otland/forgottenserver

A free and open-source MMORPG server emulator written in C++ - otland/forgottenserver

github.com

It seems like a short time, but if you multiply this time by 100 players and with an exaggerated attack speed of 100 milliseconds, we can calculate that each second the processor would be occupied for at least 220 milliseconds.
This is equivalent to approximately 20% and we are assuming that we only have 1 onHealthChange event and with some simple IF
Now imagine when you have many more events and many more systems and bad unoptimized scripts.

I did these tests with an old processor that runs at a maximum of 3.60 GHz, so these times could improve if your computer/VPS is of good quality

Consider increasing the CPU capacity of your machine and you may have better results.
But remember that the main problem is allowing players to attack like machine gun fire.

Here is a similar topic, if you want to follow it: C++ - High CPU Usage classicAttackSpeed (https://otland.net/threads/high-cpu-usage-classicattackspeed.285956/#post-2731083)

I really think my healthchange ir well made for example have only 3 codes
--achievements

function onHealthChange(creature, attacker, primaryDamage, primaryType, secondaryDamage, secondaryType, origin)
    if primaryType == COMBAT_HEALING then
        return primaryDamage, primaryType, secondaryDamage, secondaryType
    end

    local increase = function(data)
        for k, v in pairs(data) do
            local percent = (v / 100)

            --print("primaryDamage: " .. primaryDamage)
            if primaryType == k then
                primaryDamage = primaryDamage + (primaryDamage * percent)
            end
            --print("primaryDamage: " .. primaryDamage)
            if secondaryType == k then
                secondaryDamage = secondaryDamage + (secondaryDamage * percent)
            end
        end
    end

    if attacker:isPlayer() then
        --Increase Damage
        local achievements = attacker:getAchievementsInfo()
        if not next(achievements.increaseDamage) then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end
 
        increase(achievements.increaseDamage)
    end

    if creature:isPlayer() then
        --Decrease Damage
        local achievements = creature:getAchievementsInfo()
        if not next(achievements.increaseDefense) then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end    
 
        increase(achievements.increaseDefense)
    end

    return primaryDamage, primaryType, secondaryDamage, secondaryType
end

--Items dmg increase

function onHealthChange(creature, attacker, primaryDamage, primaryType, secondaryDamage, secondaryType, origin)
    if creature and attacker then
        if attacker:isPlayer() then
            local extraDmg = 0
            for i = 1,#checkslots do
                local slotitem = attacker:getSlotItem(checkslots[i])
                if slotitem and primaryType == COMBAT_ENERGYDAMAGE then
                    for k,v in pairs(specialItems) do
                        if k == slotitem.itemid then
                            -- add up all combined extraDmg here (if wearing multiple specialItems)
                            extraDmg = extraDmg + ((v.PercentDamage / 100) * primaryDamage)
                        end
                    end
                end
            end
            -- add combined extraDmg here
            primaryDamage = primaryDamage + extraDmg
        end
    end
return primaryDamage, primaryType, secondaryDamage, secondaryType
end

--Some other function

function onHealthChange(creature, attacker, primaryDamage, primaryType, secondaryDamage, secondaryType, origin)
    if attacker:isPlayer() then
        --Increase Damage
        local transformForVocation = TRANSFORMS[attacker:getVocation():getId()]
        if primaryType == COMBAT_HEALING or not transformForVocation then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end
 
        local currentTransformId = math.max(0, attacker:getStorageValue(TRANSFORM_ID_STORAGE))
        local transform = transformForVocation[currentTransformId]
        if not transform or not transform.increaseDamage then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end
 
        for k, v in pairs(transform.increaseDamage) do
            if primaryType == k then
                primaryDamage = primaryDamage * v
            end
            if secondaryType == k then
                secondaryDamage = primaryDamage * v
            end
        end  
    end
 
    if creature:isPlayer() then
        --Decrease Damage
        local transformForVocation = TRANSFORMS[creature:getVocation():getId()]
        if primaryType == COMBAT_HEALING or not transformForVocation then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end
 
        local currentTransformId = math.max(0, creature:getStorageValue(TRANSFORM_ID_STORAGE))
        local transform = transformForVocation[currentTransformId]
        if not transform or not transform.increaseDefense then
            return primaryDamage, primaryType, secondaryDamage, secondaryType
        end    
 
        for k, v in pairs(transform.increaseDefense) do
            if primaryType == k then
                primaryDamage = primaryDamage * v
            end
            if secondaryType == k then
                secondaryDamage = primaryDamage * v
            end
        end
    end
 
    return primaryDamage, primaryType, secondaryDamage, secondaryType
end

 

[Sarah Wesker]

At first glance everything seems correct.
Maybe if the specialItems table is very large it could be optimized
The local increase function could be optimized too, since building functions and tables are one of the most expensive things in Lua.
Same for getAchievementInfo

However, the best option is to improve the CPU.
You can also test, completely removing all onHealthChange events as none exist at all, and review performance.

 

[henkas]

At first glance everything seems correct.
Maybe if the specialItems table is very large it could be optimized
The local increase function could be optimized too, since building functions and tables are one of the most expensive things in Lua.
Same for getAchievementInfo

However, the best option is to improve the CPU.
You can also test, completely removing all onHealthChange events as none exist at all, and review performance.

It makes sense because i have bags in game that always stays at 100% health and a lot of people can hit them and it causes zero lag, but when it comes to a monster that has high hp and a lot of people attacks him it starts lagging, so yea probably its something with onHealthChange. And specialItems are small table like few items only in the table, local increase its same feature that increases damage and aswell super small table, getAchievementInfo is quite a big table but i dont think there is a better way of make it. And i cant get better CPU its already 8 vCores and its weird that 8 cores cant handle 15 people attacking one monster it makes no sense

 

[Fresh]

local increase its same feature that increases damage and aswell super small table

It's good that is super small table, but still it looks super bad and was written with bad habits.
Every time the onHealthChange executes (it executes if health changes, whether it's damage or healing), let's, for example, look at your super --achievements onHealthChange

In this case, we can exclude healing (because you have an early return that returns the normal state of damages onHealthChange if combat is healing).
Still... looking below this early return, it's complete garbage.
On each execution of this event, you create an local increase variable and attach an function that contains "data", and you iterate over all the elements in this "data" (whatever it is, doesn't matter if it's small or not) then it's overwriting damage, bla bla bla.. and it's going over and over again every time onHealthChange executes [damage to creature -> create local function -> insert data -> iterate over data elements -> repeat when next damage was dealt to the creature -> going again same way.., etc.].

In logic: Imagine that today you feel like you want eat 15 meals (let's consider these as the mentioned 15 players), each of these meals has some "data" meaning dishes.
Let's say that each meal (player) consists of 5 dishes (objects in the data arg passed to local function [Not to mention that it's a local function within a function that gets assigned every time onHealthChange occurs.]), which already gives you 75 "meals" (15x5). Meanwhile, you think about how much of this food there is (local increase and assigning data), and in these thoughts, you go through all the ingredients (data) that make up these 75 meals... and... this happens every time (loop) when you think about it (attack a creature), so everything I've described in this example starts from the beginning. It's normal to feel nauseous - that's how the server behaves.

...and I know that the data probably just contains types of damage and their corresponding percentages for increasing damage. If the primaryType or secondaryType matches a key in the data table, the corresponding damage is increased by the percentage specified in the data table.. but for the love of god.. everything I mentioned (and this is just the beginning) must be processed by the server.. it's bad habit and looks ekhm awful.

You didn't encourage me to read further, but think about whether it might be worth optimizing what you mentioned that "it makes no sense".

 

[henkas]

It's good that is super small table, but still it looks super bad and was written with bad habits.
Every time the onHealthChange executes (it executes if health changes, whether it's damage or healing), let's, for example, look at your super --achievements onHealthChange

In this case, we can exclude healing (because you have an early return that returns the normal state of damages onHealthChange if combat is healing).
Still... looking below this early return, it's complete garbage.
On each execution of this event, you create an local increase variable and attach an function that contains "data", and you iterate over all the elements in this "data" (whatever it is, doesn't matter if it's small or not) then it's overwriting damage, bla bla bla.. and it's going over and over again every time onHealthChange executes [damage to creature -> create local function -> insert data -> iterate over data elements -> repeat when next damage was dealt to the creature -> going again same way.., etc.].

In logic: Imagine that today you feel like you want eat 15 meals (let's consider these as the mentioned 15 players), each of these meals has some "data" meaning dishes.
Let's say that each meal (player) consists of 5 dishes (objects in the data arg passed to local function [Not to mention that it's a local function within a function that gets assigned every time onHealthChange occurs.]), which already gives you 75 "meals" (15x5). Meanwhile, you think about how much of this food there is (local increase and assigning data), and in these thoughts, you go through all the ingredients (data) that make up these 75 meals... and... this happens every time (loop) when you think about it (attack a creature), so everything I've described in this example starts from the beginning. It's normal to feel nauseous - that's how the server behaves.

...and I know that the data probably just contains types of damage and their corresponding percentages for increasing damage. If the primaryType or secondaryType matches a key in the data table, the corresponding damage is increased by the percentage specified in the data table.. but for the love of god.. everything I mentioned (and this is just the beginning) must be processed by the server.. it's bad habit and looks ekhm awful.

You didn't encourage me to read further, but think about whether it might be worth optimizing what you mentioned that "it makes no sense".

Damn and i thought everything was fine with my lua code. Well thanks

Post automatically merged: Nov 24, 2023

And how can u optimize attack speed feature tho? overall attack speed in game