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

Too many memory fragmentation when enabling MQTT #2589

Open
amarendrapramanik opened this issue May 24, 2023 · 1 comment
Open

Too many memory fragmentation when enabling MQTT #2589

amarendrapramanik opened this issue May 24, 2023 · 1 comment

Comments

@amarendrapramanik
Copy link

Device

ESP8266

Version

1.16.0-dev

Bug description

Too many memory fragmentation when enabling MQTT support. The fragmentation is very high as high as ~30% as seen in the below output.

**heap

initial: 37936, available: 23400, usable: 15488, fragmentation: 32%
+OK**

I debugged and found repeated creation of mqtt topic string creating such memory fragmentation. Is it possible to implement it with small string construct std::string or char* rather than String class construct.

Steps to reproduce

Enable MQTT support and watch for the heap usages.

Build tools used

No response

Any relevant log output (when available)

No response

Decoded stack trace (when available)

No response

@mcspr
Copy link
Collaborator

mcspr commented May 24, 2023

You version does seem out of date. Last time fragmentation: ...% string was seen in heap cmd output was before Nov 2021 - 1ca9888

String already implements main optimization from std::string - small strings do not allocate. But, iirc the size limitations, we would need to use pretty small topics to start using either version.
char* would be really tricky to use. Caller controls the lifetime right now, and we don't play around with static buffers without known length. (and don't introduce another set of bugs when switching contexts and keeping the pointer)

Can you elaborate the method that you used to test this?
Have you already changed it in your local version to verify this conclusion?

@mcspr mcspr added discussion and removed bug labels May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants