attackSpeed enabled = HIGH CPU USAGE

[Drucken]

Engine: TFS 1.5 (Nekiro Downgrade)
Tibia Version: 8.60

Is there any solution for the attack speed issue in TFS 1.5 version 8.60 downgraded by Nekiro? I ask because having the "classicAttackSpeed" boolean activated causes very high CPU usage, which causes lag on the server. When "classicAttackSpeed" is disabled, CPU usage drops by 70%.

The problem seems to come from the "doAttacking" function in the player.cpp file... This is the TFS 1.5 doAttacking:

Spoiler: doAttacking TFS 1.5 Function

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();
        bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

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

        SchedulerTask* task = createSchedulerTask(std::max<uint32_t>(SCHEDULER_MINTICKS, delay), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
        if (!classicSpeed) {
            setNextActionTask(task, false);
        } else {
            g_scheduler.addEvent(task);
        }

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

And here is another type of "doAttacking" from OTX 2 (based on TFS 0.3.7, older version), I'm only putting it here because it's done differently:

Spoiler: doAttacking OTX 2 Function

void Player::doAttacking(uint32_t)
{
    uint32_t attackSpeed = getAttackSpeed();
    if(attackSpeed == 0 || (hasCondition(CONDITION_PACIFIED) && !hasCustomFlag(PlayerCustomFlag_IgnorePacification)))
    {
        lastAttack = OTSYS_TIME();
        return;
    }

    if(!lastAttack)
        lastAttack = OTSYS_TIME() - attackSpeed - 1;
    else if((OTSYS_TIME() - lastAttack) < attackSpeed)
        return;

    if(const Weapon* _weapon = g_weapons->getWeapon(weapon))
    {
        if(!g_config.getBool(ConfigManager::CLASSIC_ATTACK_SPEED) && _weapon->interruptSwing() && !canDoAction())
        {
            SchedulerTask* task = createSchedulerTask(getNextActionTime(),
                boost::bind(&Game::checkCreatureAttack, &g_game, getID()));
            setNextActionTask(task);
        }
        else
        {
            if((!_weapon->hasExhaustion() || !hasCondition(CONDITION_EXHAUST)) && _weapon->useWeapon(this, weapon, attackedCreature))
                lastAttack = OTSYS_TIME();

            updateWeapon();
        }
    }
    else if(Weapon::useFist(this, attackedCreature))
        lastAttack = OTSYS_TIME();
}

I have no idea if the attack speed is better processed in OTX 2, I only know that TFS 1.5 does not have a good way of processing attacks, which causes very high CPU usage.

Someone who can help with this please
​

BUMP

 

[Svira]

Several techniques can be used to optimize this code and reduce CPU usage:

Use the modulo (%) operator to avoid calling OTSYS_TIME() every cycle of the loop. The value returned by this function changes too quickly to be used as the basis for a continuous countdown.

Checking if we need to call useWeapon() only once instead of twice. This will save time and reduce CPU usage.

Avoid creating new objects when not needed. Instead of creating a new SchedulerTask object each time, you can store it in the Player class and reuse it when required.

Removal of unnecessary conditions that can lead to wasted time and increased CPU usage. For example, the "if (!classicSpeed)" condition is executed only when classicSpeed is true, which means that the negation value is always false. This can be simplified by removing the condition altogether.

Reducing the number of function calls by removing unnecessary intermediate variables. For example, the value of the variable "result" is only ever used once. This can be simplified by doing the check directly on the condition where it is needed.

Below is the optimized code:

void Player::doAttacking(uint32_t)
{
static SchedulerTask* task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));

if (lastAttack == 0) {
lastAttack = OTSYS_TIME() - getAttackSpeed() - 1;
}

if (hasCondition(CONDITION_PACIFIED)) {
return;
}

uint32_t time = OTSYS_TIME();
uint32_t attackSpeed = getAttackSpeed();
if ((time - lastAttack) % attackSpeed != 0) {
return;
}

Item* tool = getWeapon();
const Weapon* weapon = g_weapons->getWeapon(tool);
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

bool result = weapon ? !weapon->interruptSwing() && weapon->useWeapon(this, tool, attackedCreature) : Weapon::useFist(this, attackedCreature);
if (result) {
lastAttack = time;
}

if (!classicSpeed && !canDoAction()) {
uint32_t delay = getNextActionTime();
task->setInterval(std::max<uint32_t>(SCHEDULER_MINTICKS, delay));
setNextActionTask(task, false);
} else {
g_scheduler.addEvent(task);
}


}

 

[GamerGoiano]

Try it too:

void Player::doAttacking(uint32_t)
{
if (hasCondition(CONDITION_PACIFIED)) {
return;
}

static uint32_t attackSpeed = getAttackSpeed();
static SchedulerTask* task = createSchedulerTask(std::max<uint32_t>(SCHEDULER_MINTICKS, getNextActionTime()), std::bind(&Game::checkCreatureAttack, &g_game, getID()));

if (lastAttack == 0) {
    lastAttack = OTSYS_TIME() - attackSpeed - 1;
}

if ((OTSYS_TIME() - lastAttack) % attackSpeed != 0) {
    return;
}

Item* tool = getWeapon();
const Weapon* weapon = g_weapons->getWeapon(tool);
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

bool result = weapon ? !weapon->interruptSwing() && weapon->useWeapon(this, tool, attackedCreature) : Weapon::useFist(this, attackedCreature);
if (result) {
    lastAttack = OTSYS_TIME();
}

if (!classicSpeed && !canDoAction()) {
    uint32_t delay = getNextActionTime();
    task->setInterval(std::max<uint32_t>(SCHEDULER_MINTICKS, delay));
    setNextActionTask(task, false);
} else {
    g_scheduler.addEvent(task);
}

}

 

[Drucken]

@Svira im going to test it.

@GamerGoiano your function uses the "task->setInterval", well "setInterval" this is not defined in tfs 1.5, could you share this function as well please?

 

[GamerGoiano]

@Svira im going to test it.

@GamerGoiano your function uses the "task->setInterval", well "setInterval" this is not defined in tfs 1.5, could you share this function as well please?

class SchedulerTask {
public:
    void setInterval(uint32_t interval) {
        this->interval = interval;
    }

    uint32_t getInterval() const {
        return interval;
    }

    // Other member functions and data members here...

private:
    uint32_t interval;
    // Other data members here...
};

 

[Drucken]

Apparently there is a problem in both functions @GamerGoiano @Svira , apart from the setInterval (I added it), the character literally does not attack.

 

[GamerGoiano]

void Player::doAttacking(uint32_t)
{
static SchedulerTask* task = nullptr; // Only create once
uint32_t currentTime = OTSYS_TIME();
uint32_t attackDelay = getAttackSpeed();
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

if (lastAttack == 0 || (currentTime - lastAttack) % attackDelay == 0) {
    lastAttack = currentTime;

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

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

    if (weapon) {
        bool interruptSwing = weapon->interruptSwing();

        if (!interruptSwing || (!classicSpeed && canDoAction())) {
            if (weapon->useWeapon(this, tool, attackedCreature)) {
                if (classicSpeed) {
                    if (task) {
                        g_scheduler.removeEvent(task);
                    }
                    task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));
                    g_scheduler.addEvent(task);
                }
            }
        } else {
            attackDelay = getNextActionTime();
        }
    } else {
        if (Weapon::useFist(this, attackedCreature)) {
            if (classicSpeed) {
                if (task) {
                    g_scheduler.removeEvent(task);
                }
                task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));
                g_scheduler.addEvent(task);
            }
        }
    }
}
}

 

[ForgottenNot]

Abu solution for this?

 

[kamilcioo]

Remove:
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);
No need for an extra if you never will actually use it.

Introduce variable for OTSYS_TIME since it's used twice
Use guard so it's more readable:

    uint32_t currentTime = OTSYS_TIME();
    if (lastAttack == 0) {
        lastAttack = currentTime - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((currentTime - lastAttack) < getAttackSpeed()) {
        return;
    }

 

[mano368]

Remove:
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);
No need for an extra if you never will actually use it.

Introduce variable for OTSYS_TIME since it's used twice
Use guard so it's more readable:

    uint32_t currentTime = OTSYS_TIME();
    if (lastAttack == 0) {
        lastAttack = currentTime - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((currentTime - lastAttack) < getAttackSpeed()) {
        return;
    }

how do you implement that in the 1.5 tfs downgrade?

 

[Nekiro]

great thread I learned a lot on how to optimize code, I will now remove every single if from my code and never declare variables, because its apparently very slow and causes huge cpu usage

 

[Forkz]

Does anyone have a solution to this problem?

 

[Nekiro]

Does anyone have a solution to this problem?

yes, two solutions:
1. dont use that option
2. rewrite (optimize) how attacking in tfs works, then you can use it

 

[endziu2222]

Maybe if possible in your engine. Disable classic attack mode and set up what attack speed you want in vocation.xml?

 

[Gesior.pl]

Someone asked me how to fix it.

Problem is not with CPU usage of basic weapon attacks. Problem is that 'classicAttackSpeed' algorithm is bugged and may generate infinite number of scheduler (dispatcher) events.
1. TFS calls Player::doAttacking every second for every player.
2. It creates new event g_scheduler.addEvent(task);, which calls Player::doAttacking again with getAttackSpeed() delay. So there is repeating event every 0.2 sec (with attack speed 200) and TFS still calls Player::doAttacking every second creating new event.
That way server can create infinite number of events for 1 player. 1 new event every second.

Often it all stops on if ((OTSYS_TIME() - lastAttack) >= getAttackSpeed()) { - player cannot attack as fast as these events execute, so if there is more than 1 dispatcher event for given player, it won't re-create it after execution (addEvent is inside that IF).
But if playeruses melee weapon and his target is not within 1 sqm, his attack will never execute and lastAttack will never update, so it will be able to re-create events, even if there is more than 1 running in same time.

How to reproduce:
1. Add in void Player::doAttacking(uint32_t) (or watch checkCreatureAttack executions per second using OTS Stats dispatcher.log):

std::cout << OTSYS_TIME() << " doAttacking " << name << std::endl;

2. Turn on classic attack in config.lua
3. Login 2 players few SQM from each other.
4. Attack one player by melee weapon - it will work even with normal attack speed '2000'.
5. Watch how number of doAttacking (or checkCreatureAttack) executions per second grows over time.
You can make it go high faster by running around. Each step of attacked/attacking player adds 1 extra event.

How to fix:
1. In player.h under:

uint32_t walkTaskEvent = 0;

add:

uint32_t classicAttackEvent = 0;

2. In player.cpp in function void Player::doAttacking(uint32_t) replace:

g_scheduler.addEvent(task);

with:

g_scheduler.stopEvent(classicAttackEvent);
classicAttackEvent = g_scheduler.addEvent(task);

It will limit number of checkCreatureAttack events to 1 per player.

Bug submited on github: classicAttackSpeed makes server go 100% CPU · Issue #4676 · otland/forgottenserver (https://github.com/otland/forgottenserver/issues/4676)

 

[Nekiro]

Someone asked me how to fix it.

Problem is not with CPU usage of basic weapon attacks. Problem is that 'classicAttackSpeed' algorithm is bugged and may generate infinite number of scheduler (dispatcher) events.
1. TFS calls Player::doAttacking every second for every player.
2. It creates new event g_scheduler.addEvent(task);, which calls Player::doAttacking again with getAttackSpeed() delay. So there is repeating event every 0.2 sec (with attack speed 200) and TFS still calls Player::doAttacking every second creating new event.
That way server can create infinite number of events for 1 player. 1 new event every second.

Often it all stops on if ((OTSYS_TIME() - lastAttack) >= getAttackSpeed()) { - player cannot attack as fast as these events execute, so if there is more than 1 dispatcher event for given player, it won't re-create it after execution (addEvent is inside that IF).
But if playeruses melee weapon and his target is not within 1 sqm, his attack will never execute and lastAttack will never update, so it will be able to re-create events, even if there is more than 1 running in same time.

How to reproduce:
1. Add in void Player::doAttacking(uint32_t) (or watch checkCreatureAttack executions per second using OTS Stats dispatcher.log):

std::cout << OTSYS_TIME() << " doAttacking " << name << std::endl;

2. Turn on classic attack in config.lua
3. Login 2 players few SQM from each other.
4. Attack one player by melee weapon - it will work even with normal attack speed '2000'.
5. Watch how number of doAttacking (or checkCreatureAttack) executions per second grows over time.
You can make it go high faster by running around. Each step of attacked/attacking player adds 1 extra event.

How to fix:
1. In player.h under:

uint32_t walkTaskEvent = 0;

add:

uint32_t classicAttackEvent = 0;

2. In player.cpp in function void Player::doAttacking(uint32_t) replace:

g_scheduler.addEvent(task);

with:

g_scheduler.stopEvent(classicAttackEvent);
classicAttackEvent = g_scheduler.addEvent(task);

It will limit number of checkCreatureAttack events to 1 per player.

Bug submited on github: classicAttackSpeed makes server go 100% CPU · Issue #4676 · otland/forgottenserver (https://github.com/otland/forgottenserver/issues/4676)

want to submit a pr?

 

[Gesior.pl]

want to submit a pr?

I made PR for TFS master: Fix classicAttackSpeed makes server lag (#4676) by gesior · Pull Request #4677 · otland/forgottenserver (https://github.com/otland/forgottenserver/pull/4677)

 

[Error 502]

should this fix also remove this note in the PR?

forgottenserver/config.lua.dist at master · otland/forgottenserver

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

github.com

 

[Gesior.pl]

should this fix also remove this note in the PR?

forgottenserver/config.lua.dist at master · otland/forgottenserver

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

github.com

View attachment 84552

classicAttackSpeed enables fast-attack. Without classic attack speed, your character will attack 1 time per second, even if you set attack speed of vocation to 10 (100x per second).
Fast attack may cause high CPU usage with 100+ players online. Especially, if you add some wand with area damage like GFB (using .lua script for weapon) and set attack speed to 100 (0.1 sec) on 'evo style' OTS.

 

[Marko999x]

classicAttackSpeed enables fast-attack. Without classic attack speed, your character will attack 1 time per second, even if you set attack speed of vocation to 10 (100x per second).
Fast attack may cause high CPU usage with 100+ players online. Especially, if you add some wand with area damage like GFB (using .lua script for weapon) and set attack speed to 100 (0.1 sec) on 'evo style' OTS.

How was 0.4 possible to handle fast attack without getting cpu problems at all?