• There is NO official Otland's Discord server and NO official Otland's server list. The Otland's Staff does not manage any Discord server or server list. Moderators or administrator of any Discord server or server lists have NO connection to the Otland's Staff. Do not get scammed!

TFS 1.X+ Help with recent memory commit logs

ralke

(҂ ͠❛ ෴ ͡❛)ᕤ
Joined
Dec 17, 2011
Messages
1,528
Solutions
27
Reaction score
871
Location
Santiago - Chile
GitHub
ralke23
Twitch
ralke23
Hi there! I'm seeking with @Itutorial if someone can provide logs of recent fixes for memory leaks on theforgottenserver. I forked my own server from nekiro's achieved repository (TFS 1.5 downgrades) and started from that point. So I missed the most of recent TFS optimizations. We have noticed some differences on our engines, where mine has more memory usage than his newer engine (10.98 with newest sources)

I'm aware that this two should be applied.
Cleanup & organize includes and PCH (#4019) · otland/forgottenserver@c359649 (https://github.com/otland/forgottenserver/commit/c3596491a3e4887872cb2d74e4d76dea7160b5c7)
Move all system & boost headers to PCH (#4022) · otland/forgottenserver@f80456e (https://github.com/otland/forgottenserver/commit/f80456ea113634f58313d2e3b8705fb12999a58b)
It is possible it fixed some loops in includes and stuff which could cause more memory usage.

Which other commits should I look for?
Also need to mention that I still use old vcpkg to compile (boost 143), that corresponds to the latest nekiro's build.

Regards, and thanks in advance!
 
Last edited:
Sorry for the delay in responding, what happens is that you are showing a screenshot with TFS using 2GB+ of RAM as if it were the engine's fault.

The changes that were made, or the vast majority of them, are based on improving performance with respect to CPU usage, for example using string_view instead of classic strings,
is that there is no need to create temporary variables that contain a copy of the string that is passed from one method to another, it is much faster for the CPU to see the string using a pointer than to constantly copy the string in each method.

If your screenshot has nothing to do with RAM then is 3.5% CPU too much for you?

Regarding RAM memory, there can be many things that cause it to increase, such as filling tables with immense amounts of data and not cleaning the tables, creating objects and not deleting them from memory, like the NPC problem that existed before.


I'm not saying that TFS is 100% fixed, it's possible that there are still some leaks that we don't know about yet, at least not that I know of.
Thanks for pointing that NPC commit. I was wondering why NPC scripts we're loading with high ms. while loading this

Can't say more than that before testing that one, but would be nice if you get us some more to have present! Thanks a lot for the help as always 💯
 
Sorry for the delay in responding, what happens is that you are showing a screenshot with TFS using 2GB+ of RAM as if it were the engine's fault.

The changes that were made, or the vast majority of them, are based on improving performance with respect to CPU usage, for example using string_view instead of classic strings,
is that there is no need to create temporary variables that contain a copy of the string that is passed from one method to another, it is much faster for the CPU to see the string using a pointer than to constantly copy the string in each method.

If your screenshot has nothing to do with RAM then is 3.5% CPU too much for you?

Regarding RAM memory, there can be many things that cause it to increase, such as filling tables with immense amounts of data and not cleaning the tables, creating objects and not deleting them from memory, like the NPC problem that existed before.


I'm not saying that TFS is 100% fixed, it's possible that there are still some leaks that we don't know about yet, at least not that I know of.
thankyou im reviewing this in order to apply correctly to my sources, have had no luck maybe you have a repository with this commit applied? if so could you hare with me please? havef been struggling to implement this
Post automatically merged:

Need specific problems to be able to help, even screenshots or copies of the lines in question to view them to be able to help. I was able to add that commit without problem, but its like @ralke said, you have to make a decision with each piece of code you alter/add/remove. You must be able to discern what is needed and not needed. For me, I remember specifically it containing code changes for stuff that had to do with podium, which is irrelevant and not in my codebase, so try to make sure you do as ralke suggested and as I am also suggesting, when applying each line, determine if there are any differences to consider, and then consider the code, and how you found it, just "copy and paste" searches are not valid enough, make sure the code you are adding/deleting/altering is in the same exact function (remember c++ has overloads, there could be multiple functions with same name), its logic is still applicable, and things of that nature, this will be majorly beneficial in preventing the errors to start, and then if you do get some, you will probably know where you suspect there could have been an issue.
im having problem at fileloader.h
specificaly here
Lua:
class PropStream
{
    public:
        void init(const char* a, size_t size) {
            p = a;
            end = a + size;
        }

        size_t size() const {
            return end - p;
        }

        template <typename T>
        bool read(T& ret) {
            if (size() < sizeof(T)) {
                return false;
            }

            memcpy(&ret, p, sizeof(T));
            p += sizeof(T);
            return true;
        }

        bool readString(std::string& ret) {
            uint16_t strLen;
            if (!read<uint16_t>(strLen)) {
                return false;
            }

            if (size() < strLen) {
                return false;
            }

            char* str = new char[strLen + 1];
            memcpy(str, p, strLen);
            str[strLen] = 0;
            ret.assign(str, strLen);
            delete[] str;
            p += strLen;
            return true;
        }

        bool skip(size_t n) {
            if (size() < n) {
                return false;
            }

            p += n;
            return true;
        }

    private:
        const char* p = nullptr;
        const char* end = nullptr;
};
my code is a bit diferent
and that is causing error here
Code:
bool IOMap::parseMapDataAttributes(OTB::Loader& loader, const OTB::Node& mapNode, Map& map,
    const std::filesystem::path& fileName)
{
    PropStream propStream;
    if (!loader.getProps(mapNode, propStream)) {
        setLastErrorString("Could not read map data attributes.");
        return false;
    }

    uint8_t attribute;
    while (propStream.read<uint8_t>(attribute)) {
        switch (attribute) {
        case OTBM_ATTR_DESCRIPTION: {
            auto [mapDescription, ok] = propStream.readString();
            if (!ok) {
                setLastErrorString("Invalid description tag.");
                return false;
            }
            break;
        }

        case OTBM_ATTR_EXT_SPAWN_FILE: {
            auto [spawnFile, ok] = propStream.readString();
            if (!ok) {
                setLastErrorString("Invalid spawn tag.");
                return false;
            }

            map.spawnfile = fileName.parent_path() / spawnFile;
            break;
        }

        case OTBM_ATTR_EXT_HOUSE_FILE: {
            auto [houseFile, ok] = propStream.readString();
            if (!ok) {
                setLastErrorString("Invalid house tag.");
                return false;
            }

            map.housefile = fileName.parent_path() / houseFile;
            break;
        }

        default:
            setLastErrorString("Unknown header node.");
            return false;
        }
    }
    return true;
}
the problem begins when i change
Lua:
//bool readString(std::string& ret) {
to
std::pair<std::string_view, bool> readString()
@Codinablack
also at some point had to convert
this
Code:
class IOMap
{
    static Tile* createTile(Item*& ground, Item* item, uint16_t x, uint16_t y, uint8_t z);

public:
    bool loadMap(Map* map, const std::filesystem::path& fileName);

    /* Load the spawns
     * \param map pointer to the Map class
     * \returns Returns true if the spawns were loaded successfully
     */
    static bool loadSpawns(Map* map)
    {
        if (map->spawnfile.empty()) {
            // OTBM file doesn't tell us about the spawnfile, lets guess it is mapname-spawn.xml.
            map->spawnfile = g_config.getString(ConfigManager::MAP_NAME);
            map->spawnfile += "-spawn.xml";
        }

        return map->spawns.loadFromXml(map->spawnfile);
    }

    /* Load the houses (not house tile-data)
     * \param map pointer to the Map class
     * \returns Returns true if the houses were loaded successfully
     */
    static bool loadHouses(Map* map)
    {
        if (map->housefile.empty()) {
            // OTBM file doesn't tell us about the housefile, lets guess it is mapname-house.xml.
            map->housefile = g_config.getString(ConfigManager::MAP_NAME);
            map->housefile += "-house.xml";
        }

        return map->houses.loadHousesXML(map->housefile);
    }

    const std::string& getLastErrorString() const { return errorString; }

    void setLastErrorString(std::string error) { errorString = error; }

to this, don't know why or if it's correct. but the previous code was giving me problem
like in here
Code:
return map->spawns.loadFromXml(map->spawnfile);
and here
Code:
    return map->houses.loadHousesXML(map->housefile);
Lua:
class IOMap
{
    static Tile* createTile(Item*& ground, Item* item, uint16_t x, uint16_t y, uint8_t z);

public:

    bool loadMap(Map* map, const std::filesystem::path& fileName);

    /* Load the spawns
     * \param map pointer to the Map class
     * \returns Returns true if the spawns were loaded successfully
     */
    static bool loadSpawns(Map* map)
    {
        if (map->spawnfile.empty()) {
            // OTBM file doesn't tell us about the spawnfile, let's guess it is mapname-spawn.xml.
            map->spawnfile = g_config.getString(ConfigManager::MAP_NAME);
            map->spawnfile += "-spawn.xml";
        }

        // Convert std::filesystem::path to std::string
        std::string spawnfilePath = map->spawnfile.string();

        // Call loadFromXml() with the converted std::string
        return map->spawns.loadFromXml(spawnfilePath);
   
    }

    /* Load the houses (not house tile-data)
     * \param map pointer to the Map class
     * \returns Returns true if the houses were loaded successfully
     */
    static bool loadHouses(Map* map)
    {
        if (map->housefile.empty()) {
            // OTBM file doesn't tell us about the housefile, let's guess it is mapname-house.xml.
            map->housefile = g_config.getString(ConfigManager::MAP_NAME);
            map->housefile += "-house.xml";
        }

        // Convert std::filesystem::path to std::string
        std::string housefilePath = map->housefile.string();

        // Call loadHousesXML() with the converted std::string
        return map->houses.loadHousesXML(housefilePath);
    }

    const std::string& getLastErrorString() const { return errorString; }

    void setLastErrorString(std::string error) { errorString = error; }

Code:
map.cpp
1>iomapserialize.cpp
1>item.cpp
1>C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,21): error C2440: 'initializing': cannot convert from 'initializer list' to 'OTB::Loader'
1>(compiling source file '/src/iomap.cpp')
1>C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,21):
1>'OTB::Loader::Loader': no overloaded function could convert all the argument types
1>    C:\Users\slim\Documents\GitHub\pro-ot\src\fileloader.h(64,2):
1>    could be 'OTB::Loader::Loader(const std::string &,const OTB::Identifier &)'
1>        C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,21):
1>        'OTB::Loader::Loader(const std::string &,const OTB::Identifier &)': cannot convert argument 1 from 'const std::filesystem::path' to 'const std::string &'
1>            C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,23):
1>            Reason: cannot convert from 'const std::filesystem::path' to 'const std::string'
1>            C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,23):
1>            No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
1>    C:\Users\slim\Documents\GitHub\pro-ot\src\iomap.cpp(58,21):
1>    while trying to match the argument list '(const std::filesystem::path, OTB::Identifier)'
@Sarah Wesker
 
Last edited:
yes I remember that error. I had to add the commit that implements a replacement in boost filesystem with std filesystem at same time to resolve @johnsamir
Post automatically merged:

These changes here:

Post automatically merged:

I think at that point I had ran into the issue still, and the last thing needed was including a header somewhere, I hope this helps
Post automatically merged:

1713756450637.png
 
Sorry for the delay in responding, what happens is that you are showing a screenshot with TFS using 2GB+ of RAM as if it were the engine's fault.

The changes that were made, or the vast majority of them, are based on improving performance with respect to CPU usage, for example using string_view instead of classic strings,
is that there is no need to create temporary variables that contain a copy of the string that is passed from one method to another, it is much faster for the CPU to see the string using a pointer than to constantly copy the string in each method.

If your screenshot has nothing to do with RAM then is 3.5% CPU too much for you?

Regarding RAM memory, there can be many things that cause it to increase, such as filling tables with immense amounts of data and not cleaning the tables, creating objects and not deleting them from memory, like the NPC problem that existed before.


I'm not saying that TFS is 100% fixed, it's possible that there are still some leaks that we don't know about yet, at least not that I know of.
NPC memory leak fix applied ^^ Fix Memory Leak from Npc events · ralke23/Greed-TFS-1.5-Downgrades@86ccd15 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/86ccd1524980fbc1149da33fefe08f09e0b3fc94)
 
yes I remember that error. I had to add the commit that implements a replacement in boost filesystem with std filesystem at same time to resolve @johnsamir
Post automatically merged:

These changes here:

Post automatically merged:

I think at that point I had ran into the issue still, and the last thing needed was including a header somewhere, I hope this helps
Post automatically merged:

View attachment 84029
already had it like that
amespace fs = std::filesystem;
what else did you add' i have all commits posted in here except for the one that im trying to add

@ralke did you succed applying this one? Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f) im facing lots of errors i cherry pick the code
edit finally succced applying this otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f) hope no get problems at login
Post automatically merged:

most of the RAM is used by the map, you should try with the default map and then with your map, and you will notice that there is a big difference
have added a thread Problem loading map after applying a commit Use string views to avoid copying strings (#4079) (https://otland.net/threads/problem-loading-map-after-applying-a-commit-use-string-views-to-avoid-copying-strings-4079.288753/#post-2750771) wonder if you could help me
 
Last edited:
already had it like that
amespace fs = std::filesystem;
what else did you add' i have all commits posted in here except for the one that im trying to add

@ralke did you succed applying this one? Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f) im facing lots of errors i cherry pick the code
edit finally succced applying this otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f) hope no get problems at login
Post automatically merged:


have added a thread Problem loading map after applying a commit Use string views to avoid copying strings (#4079) (https://otland.net/threads/problem-loading-map-after-applying-a-commit-use-string-views-to-avoid-copying-strings-4079.288753/#post-2750771) wonder if you could help me
Here's your answer my friend.
Codinablack said:
but its like @ralke said, you have to make a decision with each piece of code you alter/add/remove. You must be able to discern what is needed and not needed.
ralke said:
I stopped for a minute and started to think about it. See the code that is changed there, why merging it? Instead try to merge all those that is related to creature's path, or memory itself while running (because I had the leak while running).

Don't want to cheer you down. But you have made a back-up right? Use Git Desktop to revert the commit and start again without that one. Because as I posted here TFS 1.X+ - Help with recent memory commit logs (https://otland.net/threads/help-with-recent-memory-commit-logs.288650/#post-2750688) I haven't used that commit.

But overall. It's really good to see you trying to merge it, indeed, is a lot of code to merge (I already did the merge and reverted it, what a headache I had with it) and if you have success will be a nice achievement. Anyways, being 100% honest I don't think that filesystem commit is the key here. Seek for commits more related to what you're seeking, such as the NPCs one provided by Sarah and the full list of commits I left you before.

PS. Out of coding and post context, I really had to left PC for a while, like a day or two, after failing over and over at this. Just leave the work a little bit take a rest and keep going. You really need to tell yourself "what is possible to do with all this, and what i'm going to discard". With this i'm trying to say that probably a huge realmap with 99999999x9999999 dimensions won't work as a start (since we're trying to have a nice server, with a nice performance, with a low budget, right?).
 
Last edited:
@johnsamir

Fully implement the changes from string view, and then to eliminate the errors that were being caused in iomap, here is the solution. I forgot to include the additional PR #'s for when I did that one, and it might not even been the same, but I remembered that being an issue so when I got time today, I looked at my code and here is the fix.

1713854743384.png

the lines you are having errors in iomap with just need to have the .string() added to them to resolve the issues, so long as everything else from the string view PR is properly implemented, and so was the change from boost filesystem to std filesystem, which you said you already have done.
Post automatically merged:

I'm looking at your code and I see you already implemented a manual fix by doing a copy conversion.. so I don't understand how it can still be causing problems. I would suggest doing like ralke said, reset to before you tried to do the commit and start over.
 
@johnsamir

Fully implement the changes from string view, and then to eliminate the errors that were being caused in iomap, here is the solution. I forgot to include the additional PR #'s for when I did that one, and it might not even been the same, but I remembered that being an issue so when I got time today, I looked at my code and here is the fix.

View attachment 84044

the lines you are having errors in iomap with just need to have the .string() added to them to resolve the issues, so long as everything else from the string view PR is properly implemented, and so was the change from boost filesystem to std filesystem, which you said you already have done.
Post automatically merged:

I'm looking at your code and I see you already implemented a manual fix by doing a copy conversion.. so I don't understand how it can still be causing problems. I would suggest doing like ralke said, reset to before you tried to do the commit and start over.
ive done it almost two times till now. ill try to do it again another day im sick of this at this point hehe
i compiled it today but im facing problems in order to load the map, i think that there's no need to re do everything again just make a research at protollogin or protocolgame maybe otserv.cpp fileloader in order to get it working( noticed that those files were not equal to mine and had code as tokens etc in order to log in, things which are not requeried in 8.6
thanks for the advices

the lines you are having errors in iomap with just need to have the .string() added to them to resolve
.
the thing is that the code it's not written like that in the main repo. ofcourse i know and obviously i did it because the compiler was not allowing me to compile things in the main sintax it was the weirdest thing
happened something here
Lua:
//setText(text);
            setText(std::string(text));
well whatever ill research and fix this another day
have to do it almost manually because with cherry pick does not matter if i used mt edited files or the files from the repo it alwasy gave me errors even when there were almost no editions in the libraries used, maybe i missed an older commit that makes things fit . don't know but had to apply almost 4 editions after the cherry pick
Untitled.png
 

Attachments

@johnsamir

Fully implement the changes from string view, and then to eliminate the errors that were being caused in iomap, here is the solution. I forgot to include the additional PR #'s for when I did that one, and it might not even been the same, but I remembered that being an issue so when I got time today, I looked at my code and here is the fix.

View attachment 84044

the lines you are having errors in iomap with just need to have the .string() added to them to resolve the issues, so long as everything else from the string view PR is properly implemented, and so was the change from boost filesystem to std filesystem, which you said you already have done.
Post automatically merged:

I'm looking at your code and I see you already implemented a manual fix by doing a copy conversion.. so I don't understand how it can still be causing problems. I would suggest doing like ralke said, reset to before you tried to do the commit and start over.
how you fixed this pieces of code? @Codinablack
Lua:
query << "`conditions` = " << db.escapeString(propWriteStream.getStream()) << ',';

db.escapeString(propWriteStream.getStream())))) {
db.escapeString(propWriteStream.getStream())))) {

Lua:
query << "`conditions` = " << db.escapeString(propWriteStream.getStream()) << ',';{
for the first line i did this
Lua:
    size_t conditionsSize;
    const char* conditions = propWriteStream.getStream(conditionsSize);
    query << "`conditions` = " << db.escapeString(std::string(conditions, conditionsSize)) << ',';
dont know if it ok how you wrote it? could you share pls?
have not achieve these db.escapeString(propWriteStream.getStream())))) {
 
Last edited:
1713984396336.png
how you fixed this pieces of code? @Codinablack
Lua:
query << "`conditions` = " << db.escapeString(propWriteStream.getStream()) << ',';

db.escapeString(propWriteStream.getStream())))) {
db.escapeString(propWriteStream.getStream())))) {

Lua:
query << "`conditions` = " << db.escapeString(propWriteStream.getStream()) << ',';{
for the first line i did this
Lua:
    size_t conditionsSize;
    const char* conditions = propWriteStream.getStream(conditionsSize);
    query << "`conditions` = " << db.escapeString(std::string(conditions, conditionsSize)) << ',';
dont know if it ok how you wrote it? could you share pls?
have not achieve these db.escapeString(propWriteStream.getStream())))) {
 
Thank you for the invitation to your repository i accepted it but i can't go a review it.can you send me the link repo via inbo xpls? i think im writing something wrong
edit i fixed everything except this, db.escapeString(propWriteStream.getStre havent succed with it

db.escapeString(propWriteStream.getStream())))) {

pls and thank you
 
Last edited:
Thank you for the invitation to your repository i accepted it but i can't go a review it.can you send me the link repo via inbo xpls? i think im writing something wrong
edit i fixed everything except this, db.escapeString(propWriteStream.getStre havent succed with it

db.escapeString(propWriteStream.getStream())))) {

pls and thank you
Headed to work, I'll see what I can figure out when I get home
Post automatically merged:

My evening job is only 4 hours so I'll be back shortly
 
Last edited:
Headed to work, I'll see what I can figure out when I get home
Post automatically merged:

My evening job is only 4 hours so I'll be back shortly
nvm found it via email. going to check for the commit and after it would maybe add commits to your repo im excited since is my first time as colaborator directly. thanks again
edit: the server is 8.6? asking because of the auth token
 
Last edited:
nvm found it via email. going to check for the commit and after it would maybe add commits to your repo im excited since is my first time as colaborator directly. thanks again
edit: the server is 8.6? asking because of the auth token
Sorry bro, but I only sent you a read-only invite so you could look over the PR's and see what code changes/fixes you were needing. If you wanna talk more about that or about your issue, please lets just do so in PM, so as to not keep blowing up this hijacked thread :D
 
Sorry bro, but I only sent you a read-only invite so you could look over the PR's and see what code changes/fixes you were needing. If you wanna talk more about that or about your issue, please lets just do so in PM, so as to not keep blowing up this hijacked thread :D
im cherry picking with no problems bro :D thank the code what a bit different in your code
changed this
Lua:
if (!query_insert.addRow(fmt::format("{:d}, {:d}, {:d}, {:d}, {:d}, {:s}", player->getGUID(), pid, runningId,
            item->getID(), item->getSubType(),
            db.escapeString(propWriteStream.getStream())))) {

to this
Code:
        if (!query_insert.addRow(fmt::format("{:d}, {:d}, {:d}, {:d}, {:d}, {:s}", player->getGUID(), pid, runningId, item->getID(), item->getSubType(), db.escapeString(propWriteStream.getStream())))) {


            return false;
        }

        //if (!query_insert.addRow(fmt::format("{:d}, {:d}, {:d}, {:d}, {:d}, {:s}", player->getGUID(), pid, runningId,
            //item->getID(), item->getSubType(),
            //db.escapeString(propWriteStream.getStream())))) {
            //return false;
problem was solved the code in my and your repo is totally different than in main repo that released the commit >.< but cool it worked
now gotta test if i can load the map since with previos commit i was facing problems
 
i finally succed adding this commit the cpu ussage by tfs is still high with no monster on screen no lppayers in gm just loaded mosnters on spawn npcs its about 2,700 mb is that ok or not?
 
Back
Top