You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/03/17 12:30:21 UTC

[GitHub] [incubator-nuttx] mage0811 opened a new pull request #3086: improve the memorymodularity and reduce the inforamtion explosion

mage0811 opened a new pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086


   ## Summary
   1.Move mm_heap_s related stuff to private header file.
   2.add kconfig option to control the memory manger strategy choice.
   ## Impact
   
   ## Testing
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596722070



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       tlsf is BSD, I don't think this causes any issue, at least licensing wise. 
   Have you seen any significant improvements when using tlsf? (I think this is a general discussion that we should move out of this PR)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596115894



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       How are you including customised memory managers into the build?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596654846



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       There are many possibility here:
   
   1. Integrate other heap manager policy(e.g. https://github.com/mattconte/tlsf)
   2. Integrate the debug heap manger(e.g. https://github.com/j256/dmalloc)
   
   ASAN(#3093) support on sim platform is just a demo but very useful to debug the tough memory problem with the advanced tool impossible on the real device. Actually, we have found several bugs with ASAN and will upstream in the next couple days.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r597785259



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       How about we merge this PR first? @Ouss4 and @hartmannathan .




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596654846



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       There are many possibility here:
   
   1. Integrate other heap manager policy(e.g. https://github.com/mattconte/tlsf)
   2. Integrate the debug heap manger(e.g. https://github.com/j256/dmalloc)
   
   ASAN(#3093) support on sim platform is just a demo but very useful to debug the tough memory problem with the advanced tool impossible on the real device. Actually, we have found several stable bugs with ASAN and will upstream in the next couple days.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] davids5 commented on pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#issuecomment-801166692


   @mage0811 - this is an interesting change. To be excepted it should have [all the information](https://github.com/apache/incubator-nuttx/pull/3086#issue-594668588) filled in and it this case Testing results are key.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596678182



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       Actually, we already done integration tlsf after this patch(user can select the builtin or tlsf through Kconfig), we initially has no plan to upstream this work because it depends on the 3rd party library, but if the community want a try, we can upstream our work.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r597802261



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       @xiaoxiang781216 fine with me.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596109627



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER
+	bool "Customized heap manager"
+	---help---
+		Customized memory manger policy. The build will failed

Review comment:
       ```suggestion
   		Customized memory manger policy. The build will fail
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#issuecomment-802975123


   LGTM.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596660562



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       I was expecting something that can allow integration of other managers, like tlsf, through some build tricks, like the one we do with `context`.  #3093 seems to fix only one other option.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596654846



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       There are many possibility here:
   
   1. Integrate other heap manager policy(e.g. https://github.com/mattconte/tlsf)
   2. Integrate the debug heap manger(e.g. https://github.com/j256/dmalloc)
   
   ASAN(#3093) support on sim platform is just a demo but very useful to debug the tough memory problem with the advanced tool impossible on the real device.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] mage0811 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
mage0811 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596554049



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       hi:
   please check my patch "mm: add heap policy and anta logic to support mm debug on sim platform #3093",
   this is my customize mm policy used under sim platform for sanitizers debug tool.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] hartmannathan commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596921098



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER

Review comment:
       > Actually, we already done integration tlsf after this patch(user can select the builtin or tlsf through Kconfig), we initially has no plan to upstream this work because it depends on the 3rd party library, but if the community want a try, we can upstream our work.
   
   If the license isn't compatible, maybe a Kconfig option to allow an "external" memory manager. Then the user can specify any manager, even their own custom one, as long as it provides the interfaces NuttX needs.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] mage0811 commented on a change in pull request #3086: improve the memory manager modularity and reduce the inforamtion explosion

Posted by GitBox <gi...@apache.org>.
mage0811 commented on a change in pull request #3086:
URL: https://github.com/apache/incubator-nuttx/pull/3086#discussion_r596539192



##########
File path: mm/Kconfig
##########
@@ -3,6 +3,23 @@
 # see the file kconfig-language.txt in the NuttX tools repository.
 #
 
+choice
+	prompt "Build heap manager"
+	default MM_DEFAULT_MANAGER
+
+config MM_DEFAULT_MANAGER
+	bool "Default heap manager"
+	---help---
+		NuttX original memory manager strategy.
+
+config MM_CUSTOMIZE_MANAGER
+	bool "Customized heap manager"
+	---help---
+		Customized memory manger policy. The build will failed

Review comment:
       hi,when select "MM_CUSTOMIZE_MANAGER", then the user should develop the customize mm first.
   
   please check my patch "mm: add heap policy and anta logic to support mm debug on sim platform #3093",
   this is my customize mm policy used under sim platform for sanitizers debug tool.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org