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

Custom Heap Allocation. #696

Closed
wants to merge 1 commit into from

Conversation

slaff
Copy link
Contributor

@slaff slaff commented Apr 13, 2016

Based on umm_malloc. Most of the integration code comes from esp8266/Arduino.

Advantages:
-- Better heap allocation and less fragmentation. We can use the available free heap more efficiently.

Disadvantages:
-- A bit slower than the original mem_manager functions.

@hreintke
Copy link
Contributor

@slaff :
Due to the status of Sming NONOS being replaced by SmingRTOS I am not in favor of putting this kind of major updates in.
For SmingRTOS the heap assignment is based on the Freertos implementation, not sure how this will fit in but please first align before implementing.


void system_show_malloc(void)
{
umm_info(NULL, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi do you have example output for this function call? I'm curious if this could be used for run time fragmentation monitoring

@slaff
Copy link
Contributor Author

slaff commented Apr 14, 2016

I am not in favor of putting this kind of major updates in.

@hreintke I can check how is the memory allocation done in RTOS and port the same code also for SmingRTOS.

heap assignment is based on the Freertos implementation, not sure how this will fit in

I will be surprised if it is much different than the one in NONOS. But I have to check. Anyone having an idea about this?

I'm curious if this could be used for run time fragmentation monitoring

@ADiea Because one has control over the memory management functions it is possible to log the memory allocation and should be relatively easy to implement (as done here ).

@slaff
Copy link
Contributor Author

slaff commented Apr 14, 2016

I will be surprised if it is much different than the one in NONOS. But I have to check. Anyone having an idea about this?

It turns out that the heap allocation is open source and can be read from here:
https://github.com/espressif/ESP8266_RTOS_SDK/blob/master/third_party/freertos/heap_4.c .Well, from here on only a good comparison can show the winner: FreeRTOS vs UMM.

@hreintke
Copy link
Contributor

@slaff :
Yes, that is the location where the freertos heap management is done.
And frankly, I prefer to stay with the freertos version as that is in use in close to all freertos implementations.
The question is not : who is the absloute best
But : Is there an issue with the current one which we need to solve

@patrickjahns
Copy link
Member

@slaff
I like it - but needs some more testing in order to ensure it`s stable

@hreintke
there is no issue with the current one - not in a normal way - but with frequent (re-)allocaions the memory could get more fragmented with the current implementation compard to umm

probably when running for longer times and using parts that require a lot of reallocs (using a lot of String class) we could run into more trouble. The webserver can cause quite a few reallocs / allocs and fragment the heap

@hreintke
Copy link
Contributor

@patrickjahns : @slaff :
If there is no issue with the current one, I see no reason for any update.
Only a "probably" is not enough to make a major update like this.

These kind of updates, within the hart of the underlying freertos software, should be kept to an absolute minimum.

@slaff
Copy link
Contributor Author

slaff commented Apr 14, 2016

If there is no issue with the current one, I see no reason for any update.

@hreintke Sounds reasonable to me concerning RTOS. Then why not merge this PR in NONOS? In NONOS there is an issue with the heap allocation which is easily visible and UMM, integrated in this PR, has better heap allocation. In addition the UMM integration can be switched on and off during compile time with the ENABLE_CUSTOM_HEAP directive.

@hreintke
Copy link
Contributor

@slaff :

Due to the status of Sming NONOS being replaced by SmingRTOS I am not in favor of putting this kind of major updates in.

Why should we put time and effort in a major update in a version for which support will cease on short notice.

@ADiea
Copy link
Contributor

ADiea commented Apr 21, 2016

Still noticing some weird behavior of malloc, under investigation see attached png. The last 2 mallocs at 4154 42dc don't seem ok to me
first_mallocs

@hreintke hreintke added the Ideas label Apr 22, 2016
@ADiea
Copy link
Contributor

ADiea commented Apr 28, 2016

The issue in the last post was my fault, not reporting all the allocations. I confirm ummaloc works great and should be merged

@hreintke
Copy link
Contributor

@slaff :
As I mentioned two weeks ago

Due to the status of Sming NONOS being replaced by SmingRTOS I am not in favor of putting this kind of major updates in.

Unless we are solving a very high priority issue, I have no intend to merge this in Sming

@hreintke
Copy link
Contributor

No need to merge for NONOS, closing PR

@hreintke hreintke closed this May 23, 2016
@hreintke hreintke removed the Ideas label May 23, 2016
@slaff slaff deleted the feature/umm-malloc-dev branch February 23, 2017 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants