MSRevive / MasterSwordRebirth

Continuation of Master Sword Classic/Continued.
https://msrebirth.net/
Other
9 stars 6 forks source link

msarea_monsterspawn lags the server #92

Closed SaintWish closed 1 year ago

SaintWish commented 2 years ago

It seems msarea_monsterspawn can lag the server when spawning mobs. This is probably what is causing this issue https://github.com/MSRevive/MasterSwordRebirth/issues/54

SaintWish commented 2 years ago

going up the ramp in chapel is the best way to test it, on a fresh map it'll spawn 7 boars and rats one after another

Charles445 commented 2 years ago

After some investigation, I believe that the source of new lag is the frequent call to strlen in the lines loop https://github.com/MSRevive/MasterSwordRebirth/blob/dev/game/shared/ms/script.cpp#L5008

This was changed in this commit to "fix buffer overflow issues" https://github.com/MSRevive/MasterSwordRebirth/commit/10527b0097f2514a6f21ee970d97c7d590b9d893

Reverting this GetString to the NOV2015a implementation more than doubles the performance, however if this really did fix a buffer overflow, then that needs to be addressed

BerntA commented 1 year ago

Perhaps a proper rewrite of this method would be best, or a quick refactor, seems like some of the strlen calls could be cached before the loop and just be maintained throughout the loop?

SaintWish commented 1 year ago

Perhaps a proper rewrite of this method would be best, or a quick refactor, seems like some of the strlen calls could be cached before the loop and just be maintained throughout the loop?

Yeah I'm trying to rewrite scriptfile parser to not have so many strlen calls and such to hopefully speed up the system substantially. Meanwhile @Charles445 came up with a way to cache the initial strlen which also speeds up the system.

const char* pszScriptdata_start = pszScriptData;
size_t pszScriptdata_len = 0;

if (*pszScriptData != 0)
    pszScriptdata_len = strlen(pszScriptData);

while(*pszScriptData != 0)
    {
        char cBuf[768];
        //cBuf[0] = 0;
        if (GetString(cBuf, min(pszScriptdata_len - (pszScriptData - pszScriptdata_start) + 1, sizeof(cBuf)), pszScriptData, 0, "\r\n"))
BerntA commented 1 year ago

Clever 👍

SaintWish commented 1 year ago

So rewriting the scriptfile parser improved the speed quite a bit. Was at ~120 milliseconds before @Charles445's fix, his fix brought the speed to ~60 milliseconds. Then with the new scriptfile parser the speed is now ~30 milliseconds. This is tested by going up the ramp in chapel towards the boars.

I think we can further improve the speed by rewriting the scriptline parser aswell which is what I'm going to try to do next.