TFS 1.X+ Help with recent memory commit logs

[ralke]

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!

 

[Codinablack]

(boost 143)

143 is the MSVC target platform set number, not the Boost version number. Honest mistake though.

I know one right off the bat you should see if you have

Fix Memory Leak from Npc events by MillhioreBT · Pull Request #4483 · otland/forgottenserver

Pull Request Prelude I have followed [proper The Forgotten Server code styling][code]. I have read and understood the [contribution guidelines][cont] before making this PR. I am aware that this...

github.com

That one is for memory leak in NPC's.

I don't think that the ones for the PCH and includes will affect you at all on memory. They are basically just edits that can speed up compile time.

I started with 1.4.2 for Black Tek, and have mainly pulled all my commits from the master branch, and I included the pull # as well, so you can find easily on the main branch with that number any of the commits, or just look at the way I handled that commit, as anyone who based off 1.4.2 should have similar enough code bases in comparison to main. I will send you invite on github, check your notifications.

Post automatically merged: Apr 15, 2024

I'm heading to bed atm, but when I get up, I will look more into the commits I have farmed and am getting ready to farm, to see if anything jumps out at me.

 

[Codinablack]

I am unsure what your commit list looks like, but I was thinking about it, and I think the biggest performance commits since 1.4.2 are the "replace std::bind with lambda" commit, and the "use string view" commits. Both of those commits should definitely make a difference in what you see on your server, versus someone's whose is more up to date.

 

[Itutorial]

Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

A free and open-source MMORPG server emulator written in C++ - Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

github.com

Replace std::bind with lambdas (#4017) · otland/forgottenserver@92b35aa

A free and open-source MMORPG server emulator written in C++ - Replace std::bind with lambdas (#4017) · otland/forgottenserver@92b35aa

github.com

Replace usages of str*cmp with C++ functions (#4018) · otland/forgottenserver@336ea63

A free and open-source MMORPG server emulator written in C++ - Replace usages of str*cmp with C++ functions (#4018) · otland/forgottenserver@336ea63

github.com

 

[ralke]

Thanks! I'm in process of merging. Talked to @Itutorial via discord so he can tell me if the changes were correctly added. It's a huge difference, this one specifically got me aware of how different downgrade sources are: Use string views to avoid copying strings

I almost gave up before merging, was so stressed. Thought on starting a fork of newer repostories such as Millhiore's one, but got aware by @Marko999x that those one has some issues with cip clients. So I cheer up myself and started to merge everything. Definitely the a* algorythm thread lead me to a bigger thing, I hope all this effort leads to a succesfull distribution for everyone.

143 is the MSVC target platform set number, not the Boost version number. Honest mistake though.

XD

I am unsure what your commit list looks like

I will definitely post my commit log as soon we review it with @Itutorial ^^

Again, thank you both!
Regards

 

[Codinablack]

I would also make sure you have the one that trades "std::variant for luavariant". I don't know the link off top of my head but the PR number is #3981

I noticed it while building my list of features for Black Tek Server, which I will soon share as another teaser on my thread about updated tfs 1.4. I will have the ai sort it by my three tags "CodeQuality, Enhancement, and Fix", so it would be a great point of reference for you to see what you might be missing that would be of use, and each one has the PR number from the Main branch of TFS (if applicable) so its easy for anyone to check the commit, and my work, against it.

 

[johnsamir]

i have almost all those commits applied excluding this Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f)
today was testing itutorial monster code ai, and although i have all these commits the memory usage is still high asf so might be something else :/

 

[Codinablack]

i have almost all those commits applied excluding this Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f)
today was testing itutorial monster code ai, and although i have all these commits the memory usage is still high asf so might be something else :/

View attachment 83850

The string view is very important for memory, as everytime a string is used, it is copied and creates like three allocations on the heap, with string view there are 0. So imagine copying a screenshot for example, three times on your hard drive, just to share on discord, but then never deleting any of your copies, when you could have just pressed printscreen and then paste to discord without ever saving to your hard drive...

 

[johnsamir]

The string view is very important for memory, as everytime a string is used, it is copied and creates like three allocations on the heap, with string view there are 0. So imagine copying a screenshot for example, three times on your hard drive, just to share on discord, but then never deleting any of your copies, when you could have just pressed printscreen and then paste to discord without ever saving to your hard drive...

haven't succed in adding it i've cherry pick the code, but im facing libraries errors after it and an error related to non operator etc
yet im going keep trying

 

[Codinablack]

haven't succed in adding it i've cherry pick the code, but im facing libraries errors after it and an error related to non operator etc
yet im going keep trying

show me the errors in console. Usually one error causes 16516516565 to show up or something like that, because if one thing is wrong, then other things after it are now affected due to syntax, but usually its like one line, even if it says 75 errors!

Just send it to me in DM

 

[johnsamir]

up somebody have succed in adding this commit?

Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

A free and open-source MMORPG server emulator written in C++ - Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

github.com

it's giving me problems

 

[ralke]

up somebody have succed in adding this commit?

Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

A free and open-source MMORPG server emulator written in C++ - Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

github.com

it's giving me problems

Hi! I had to face the same and then 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). I see that one is getScriptEventName() related, this way I supposed that this was on startUp stuff. And we're seeking for those related to memory while running.

Since we're both trying to follow the a* pathfinding thread, I suggest to check here (see the description on the commits, there's the link to NRH-AA repositories) Commits · ralke23/Greed-TFS-1.5-Downgrades (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commits/8.60/)

And here, the NRH-AA repository itself.

Commits · NRH-AA/forgottenserver

A free and open-source MMORPG server emulator written in C++ - Commits · NRH-AA/forgottenserver

github.com

I already merged latest changes, and it's running perfectly fine. And yes, I omited a few commits ;P
And this, it's really usefull as I see TFS 1.X+ - Possible infinite loop at actions/lib/actions.lua (https://otland.net/threads/possible-infinite-loop-at-actions-lib-actions-lua.288733/#post-2750625)

The string view is very important for memory, as everytime a string is used, it is copied and creates like three allocations on the heap, with string view there are 0. So imagine copying a screenshot for example, three times on your hard drive, just to share on discord, but then never deleting any of your copies, when you could have just pressed printscreen and then paste to discord without ever saving to your hard drive...

I don't doubt that you are right but doing a clean merge of that one was hard (at least for me). Idk if there's easier methods for it. I will only bet for an organized repository since is the only way to have an order without screwing everything xD

For the moment there's no urgency anymore, since my memory started to work again. But definitly we could look further into this, to get a good 8.6 downgrade aswell (I see you guys work on 10.98).

Regards!

 

[Sarah Wesker]

i have almost all those commits applied excluding this Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f)
today was testing itutorial monster code ai, and although i have all these commits the memory usage is still high asf so might be something else :/

View attachment 83850

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

 

[ralke]

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

Indeed, I cut a big piece of mine and it helped a lot haha ^^

 

[johnsamir]

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

what u mean? im talking about add that commit what's has to do the commit with maps?

 

[ralke]

what u mean? im talking about add that commit what's has to do the commit with maps?

Commit has to do with maps, for example, depeding on your map size all that is load at bool IOMap::loadMap(Map* map, const std::string& fileName) is triggered on start-up. At this line, on the commit you attached Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f#diff-d5deb95ed188688fc3a0a4831fcad9a7ec80148e9f2452f7afb5529063494508R154) there's a reference to map, you can dig on that and you'll get more and more.

So I suppose that changing string views affects memory performance directly, because they use a more optimized method to load and get those resources (remember that map holds actions and unique ids, etc, etc...).

 

[johnsamir]

Commit has to do with maps, for example, depeding on your map size all that is load at bool IOMap::loadMap(Map* map, const std::string& fileName) is triggered on start-up. At this line, on the commit you attached Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7 (https://github.com/otland/forgottenserver/commit/a295ea7b8881f73160500757a2f860dbc64bbf4f#diff-d5deb95ed188688fc3a0a4831fcad9a7ec80148e9f2452f7afb5529063494508R154) there's a reference to map, you can dig on that and you'll get more and more.

So I suppose that changing string views affects memory performance directly, because they use a more optimized method to load and get those resources (remember that map holds actions and unique ids, etc, etc...).

mm i get it now, im trying to apply this but im facing problems, has somebody of you guys succed applying this?
@ralke @Sarah Wesker

 

[ralke]

I'll try to do it the easier as possible. Do the following to merge (some commits will tell you to do this... and then revert that... just don't delete & go, sometimes use // to comment code in case you probably have to add it later):

The following commit logs must be applied from achieved nekiro downgrade repository (8.6, 8.0, whatever).

1. Gigastar A* pathfinding Gigastar A* Pathfinding · ralke23/Greed-TFS-1.5-Downgrades@45f9793 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/45f9793faa63201db903bc79d724bae80c9b6f46)
2. Gigastar pathfinding call Gigastar Pathfinding Call · ralke23/Greed-TFS-1.5-Downgrades@eb642bf (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/eb642bf87f76c600479abdc16201af816183dead)
3. Method set master position Method setMasterPosition · ralke23/Greed-TFS-1.5-Downgrades@8f2d82a (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/8f2d82ad839cde1612cc8ac6dc9c8f0ecbc855d6)
4. Update follow path Update Follow Path · ralke23/Greed-TFS-1.5-Downgrades@3077e8f (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/3077e8f539a216cbbbaa2d056c35427057aea482)
5. PLAYER_SEARCHDIST PLAYER_SEARCHDIST · ralke23/Greed-TFS-1.5-Downgrades@b73d122 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/b73d122ab47035e0cc7e6c126a6db463f42c7555)
6. getPathTo (position to pos) getPathTo (position to pos) · ralke23/Greed-TFS-1.5-Downgrades@9a440f3 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/9a440f39610b684fefc1ca8c1b1f16ab410093e1)
7. Add follow position to follow creature Add followPosition to followCreature · ralke23/Greed-TFS-1.5-Downgrades@044299d (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/044299d305d2e744d294c8799c2a86630903b24d)
8. Summon keepDistance, isInRange, distance check Summon keepDistance, isInRange, distance check. · ralke23/Greed-TFS-1.5-Downgrades@50d9f7b (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/50d9f7bc8c311c487cf840123808e7f7dc7aaeaa)
9. Corrections Corrections · ralke23/Greed-TFS-1.5-Downgrades@d105eb0 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/d105eb0d917f2814ec9408f3c8bbe2710d7b9eba)
10. 9 players stackpos bug 9 players stackpos bug · ralke23/Greed-TFS-1.5-Downgrades@4473d68 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/4473d6806ea7420bc78cf58b1fea28e991acf67d)
11. Fir3lement 0.4 goToFollowCreature Fir3lement 0.4 goToFollowCreature · ralke23/Greed-TFS-1.5-Downgrades@404c478 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/404c478dd10811a9a2275bce9e0b94c18df0923c)
12. NRH-AA Pathfinding NRH-AA Pathfinding · ralke23/Greed-TFS-1.5-Downgrades@570c4d7 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/570c4d7b3aef10166ae8ab52f87ffa167ab70415)
13. First adjustments on NRH-A Pathfinding First adjustments on NRH-A Pathfinding · ralke23/Greed-TFS-1.5-Downgrades@d5670ec (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/d5670ec78becd3ec9fa6eb60b4304628381ad78c)
14. Optimize pathfinding by NRH-AA Optimize pathfinding by NRH-AA · ralke23/Greed-TFS-1.5-Downgrades@efdd636 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/efdd636193d4a50df88733811631cf0afa161c92)
15. Path viewport check, fix to blocked sight of monsters (needs review, it causes lag and memory increase) Path viewport check, fix to blocked sight of monsters (needs review, … · ralke23/Greed-TFS-1.5-Downgrades@d001657 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/d0016579d68c06570487444e62e29137bf3c5eed)
16. NRA-AA fixing memory leaks NRA-AA fixing memory leaks · ralke23/Greed-TFS-1.5-Downgrades@4fc0dfa (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/4fc0dfa394249ca212690d0e62d456fa6433adb6)
17. NRH-AA latest changes NRH-AA latest changes · ralke23/Greed-TFS-1.5-Downgrades@81f7784 (https://github.com/ralke23/Greed-TFS-1.5-Downgrades/commit/81f7784fccd6ebd074e4b22b6f72488c5eda6123)

Can't be more easy to follow than this. Remember. You need to use achieved Nekiro distribution.
Regards!

 

[Codinablack]

up somebody have succed in adding this commit?

Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

A free and open-source MMORPG server emulator written in C++ - Use string views to avoid copying strings (#4079) · otland/forgottenserver@a295ea7

github.com

it's giving me problems

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.

 

[Sarah Wesker]

what u mean? im talking about add that commit what's has to do the commit with maps?

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.