Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTBM RAM usage #624

Closed
2 of 5 tasks
gesior opened this issue Nov 12, 2023 · 11 comments
Closed
2 of 5 tasks

OTBM RAM usage #624

gesior opened this issue Nov 12, 2023 · 11 comments
Labels
Priority: Low Minor impact Stale Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@gesior
Copy link
Contributor

gesior commented Nov 12, 2023

Priority

Low

Area

  • Data
  • Source
  • Docker
  • Other

What happened?

I've tried to generate full minimap for TFS using OTClient Redemption. Typed in Terminal:

g_game.setClientVersion(1098)
g_things.loadDat('things/1098/Tibia.dat')
g_sprites.loadSpr('things/1098/Tibia.spr')
g_things.loadOtb('items.otb')
g_map.loadOtbm('forgotten.otbm')
g_minimap.saveOtmm('testmap.otmm')

and RAM usage went to 550 MB - with edubart/OTCv8 it's 374 MB.

Then tested with 115 MB .otbm map and it looks like Redemption client uses 2x more memory for OTBM:
edubart Release x64: 4244 MB RAM
OTCv8 Release x86: too big map for 32-bit
Mehah Release x64: 8672 MB RAM
Mehah Debug x64: 16651 MB RAM

I also tested this on Linux and the results are the same. Redemption client used 10 GB after loading 115 MB .otbm.

Does anyone have an idea what was changed in Redemption client that it uses 2x more RAM for OTBM?

What OS are you seeing the problem on?

Linux, Windows

Code of Conduct

  • I agree to follow this project's Code of Conduct
@gesior gesior added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Nov 12, 2023
@github-actions github-actions bot added Priority: Low Minor impact Status: Pending Test This PR or Issue requires more testing labels Nov 12, 2023
@4drik
Copy link

4drik commented Nov 12, 2023

I bet this is the result of getting rid of:
stdext::dynamic_storage m_attribs;

I'm using Naruto Story client and I haven't tested Mehah's client long time, but I've been following Mehah's changes. From what I remember, he got rid of attributes in thingType, which had long seemed suspicious to me in terms of RAM. I often see reports on Discords that something is not working, either in OTCv8 or Redemption.
With the current repositories, you can create a stable, better than before client in just a few days.

@mehah
Copy link
Owner

mehah commented Nov 13, 2023

@gesior

I believe that the high memory consumption is due to the use of phmap::flat_hash_map, try replacing it with std::unordered_map and see if the consumption decreases.

https://github.com/mehah/otclient/blob/7da55d913b7c420436e727ecabf2dd2d13aeebd7/src/client/map.h#L297C5-L297C9

@gesior
Copy link
Contributor Author

gesior commented Nov 15, 2023

@mehah
Tested with std::unordered_map and std::map (as in OTCv8). RAM usage is the same, still 2x higher.
Maybe it's somehow related to changes in structures related to handling 13+ client files.

One thing I've noticed is that .otmm files generated by Redemption are 5-10% bigger than files generated by other OTCs.
Also tried to save 158.4 MB .otbm loaded in Redemption. Saved .otbm has 165.9 MB.
Load -> save functions should not modify map structure. I will try to detect changes in .otbm using OTBM2JSON on weekend and post more information about it.

I bet this is the result of getting rid of: stdext::dynamic_storage m_attribs;

I'm using Naruto Story client and I haven't tested Mehah's client long time, but I've been following Mehah's changes. From what I remember, he got rid of attributes in thingType, which had long seemed suspicious to me in terms of RAM. I often see reports on Discords that something is not working, either in OTCv8 or Redemption. With the current repositories, you can create a stable, better than before client in just a few days.

Only commit with thingType I found is 940aef8
Is this the change you mentioned?

@4drik
Copy link

4drik commented Nov 24, 2023

@gesior
Let me show you an example:
Edubart:
bool isGround() { return m_attribs.has(ThingAttrGround); }
Mehah:
bool isGround() { return (m_flags & ThingFlagAttrGround); }

uint64_t m_flags{ 0 };
always takes 64 bits of memory. Each object that contains m_flags will have 64 bits of memory reserved for these flags.

stdext::dynamic_storage<uint8> m_attribs; may take up less memory if it only stores a few attributes. Each attribute is a uint8 which occupies 8 bits of memory, so using flags would make sense if objects always had ~8 attributes.

@mehah
Copy link
Owner

mehah commented Nov 24, 2023

std::dynamic_storage (note: this is just struct size )
image

uint64
image

you are comparing a simple 64bit integer with a class, with some methods, a vector and 'any'

anyway,

@gesior , I have some priorities ahead, as soon as I have time off, I'll pay attention to this, especially because I want to transform the Client into a map editor, so it needs to consume little memory.

@4drik
Copy link

4drik commented Nov 24, 2023

I'm sorry I misled you and thank you for the correction

@conde2
Copy link
Collaborator

conde2 commented Nov 26, 2023

Some things added recently that probably increased RAM usage:

Tile now has a WidgetPtr on it
image

All things and TILE are now a sub class of AttachableObject, this class has some extra objects in it:
image

What can be done is add a unique_ptr pointing to a new object of Attachable Object in Thing and Tile. Then when we need to deal with attached stuff we access the class through this pointer. TFS does something similar with item and attributes.

@gesior
Copy link
Contributor Author

gesior commented Nov 28, 2023

@conde2
I will try to replace these variables with nullptr or pointer to object outside class in 'has/get' methods and check how much RAM it uses.
I don't have access to PC able to compile OTC now. I will test it in 2 weeks and post results.

Anyway, I must first test if https://github.com/gesior/otclient_mapgen code moved to Mehah will be able to render map images using 13+ client files and no .otb file or Mehah code for 13+ reads only features required by client, ignoring data required to load .otbm.

Copy link

This issue is stale because it has been open 120 days with no activity.

@github-actions github-actions bot added the Stale label Feb 27, 2024
@mehah
Copy link
Owner

mehah commented May 11, 2024

test this commit
404e2c5

note: Perhaps consumption is still a little high, as we use shared_ptr from STL, instead of OTC Legacy.

https://github.com/edubart/otclient/blob/master/src/framework/stdext/shared_ptr.h

@mehah mehah closed this as completed May 11, 2024
@gesior
Copy link
Contributor Author

gesior commented May 30, 2024

test this commit 404e2c5

note: Perhaps consumption is still a little high, as we use shared_ptr from STL, instead of OTC Legacy.

https://github.com/edubart/otclient/blob/master/src/framework/stdext/shared_ptr.h

Looks like this commit is merged into main. I tried to compile main with FRAMEWORK_EDITOR, but it failed because of this line:

m_thingType = g_things.getThingType(m_clientId, ThingCategoryItem).get();

I removed this line and now it compiles.
Tested again:

  • otclient_mapgen (based on edubart) Release x64: 4204 MB RAM (was 4244 MB, so results are similar)
  • Mehah Release x64: 7872 MB RAM (was 8672 MB)
  • Mehah Debug x64: 14116 MB (was 16651 MB RAM)

There is some reduction. I will try to add 'map renderer' code to Mehah OTC next week and test how fast it will work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Minor impact Stale Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

No branches or pull requests

4 participants