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

feat(memory-map) allow fixed static ram and heap size boundaries (IDFGH-858) #3222

Closed
wants to merge 4 commits into from

Conversation

GautierAtWork
Copy link
Contributor

Currently the heap size is defined at link time as "the remaining ram", what is not deterministic.

Once the product is released, and a minor revision is created, it should be ensured that critical parameters of the system are not impacted.
With a non deterministic heap size, no one can guarantee that connectivity is not impacted by any code change, even a minimal innocent one.

This patch makes the static ram size configurable from Kconfig, and indirectly fixed the heap size. Any change of the heap size is fully under control.

@github-actions github-actions bot changed the title feat(memory-map) allow fixed static ram and heap size boundaries feat(memory-map) allow fixed static ram and heap size boundaries (IDFGH-858) Mar 25, 2019
@negativekelvin
Copy link
Contributor

The runtime heap allocator does not use _heap_start it only uses _bss_end so this may have no effect on heap size and only restrict dram0_0_seg length

@GautierAtWork
Copy link
Contributor Author

@negativekelvin Thanks for that. Seems it changed since the ESP-Idf version we are using.
I will have a look.

@GautierAtWork
Copy link
Contributor Author

@negativekelvin: fixed

Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

Hi @GautierAtWork ,

This seems like a good addition for increasing determinism of resource use. A couple of small notes, but mostly LGTM.

components/esp32/Kconfig Outdated Show resolved Hide resolved
components/esp32/Kconfig Outdated Show resolved Hide resolved
@projectgus projectgus requested a review from igrr April 2, 2019 06:53
Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @GautierAtWork, and apologies for the late review. Just a couple of cosmetic comments.

Also, can you please combine the commits into one and rebase onto master? Thanks.

components/esp32/Kconfig Outdated Show resolved Hide resolved
components/esp32/Kconfig Outdated Show resolved Hide resolved
@projectgus
Copy link
Contributor

Thanks for contributing this, @GautierAtWork . Thanks also for being patient while we worked through the review process.

I've squashed these commits into one and pushed them into our internal review & merge queue. The PR will be updated once the changes are merged and pushed to GitHub.

igrr pushed a commit that referenced this pull request Jun 10, 2019
@igrr
Copy link
Member

igrr commented Jul 10, 2019

Thanks for the PR @GautierAtWork, it has been cherry-picked as 542e544.

@igrr igrr closed this Jul 10, 2019
trombik pushed a commit to trombik/esp-idf that referenced this pull request Aug 9, 2019
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