Remove custom heap #486
Remove custom heap #486
Conversation
- remove all references and code related with custom heap - rework scatter files, AppEntry, FirstEntry and TinyHAL - reworked startup files to perform memory initializations - clean-up unused code
josesimoes
force-pushed
the
Remove-Custom-Heap-473
branch
from
fc1209d
to
82e276d
Compare
|
Please remove the inclusion of additional PRs that makes it impossible to roll back or process PRs independently. Each PR should be isolated and focused on doing one thing that is as small and narrowly defined as possible. |
| @@ -3,7 +3,7 @@ | |||
| // <No description> | |||
| // | |||
| // Microsoft dotNetMF Project | |||
| // Copyright �2004 Microsoft Corporation | |||
| // Copyright �2004 Microsoft Corporation | |||
There was a problem hiding this comment.
please do not alter copyright comments/notices in the code.
There was a problem hiding this comment.
To be accurate there is no change in the copyright notice. I haven't changed that obviously.
Being the copyright symbol that is different I would say that GitHub is having problems with the code page of the file it something like that.
But I'll do my best removing that difference from the file.
|
Which PR are you referring to? |
| @@ -16,8 +16,6 @@ static int s_PreHeapInitIndex = 0; | |||
|
|
|||
| //////////////////////////////////////////////////////////// | |||
|
|
|||
| HAL_DECLARE_CUSTOM_HEAP( CLR_RT_Memory::Allocate, CLR_RT_Memory::Release, CLR_RT_Memory::ReAllocate ); | |||
There was a problem hiding this comment.
Umm, hold on a sec... This is defining the managed heap APIs. The HAL_DECLARE_CUSTOM_HEAP creates a set of private_xxx APis that are inlined wrappers around the methods listed in the macro. While the entire approach to using a macro like this is dubious at best, we don't want to eliminate the functionality as intended here. The use here is intended to be the managed heap which has lots of internal requirements and assumptions for the collector to work properly.
There was a problem hiding this comment.
Are you entirely sure about that?!
The name refers to custom heap.
I have build this without this code and it went fine.
I have loaded the image on real hardware a run a test app. Creating and destroying objects. Checked the managed memory going up and down a seeing the GC kicking on. I admit it wasn't nothing fancy or exhaustive but it proved that the managed heap was working.
|
@josesimoes I'm referning to this statement:
|
|
I can remove the statement. As for PR #481 it was just merged into the repo, so I guess it wouldn't even allow a merge if I removed the respective changes... |
|
Now that it is merged you can re-base so hopefully those changes will disappear and we can focus on the heap specific changes. |
… Remove-Custom-Heap-473
|
@smaillet-ms done! |
| @@ -3,7 +3,7 @@ | |||
| // <No description> | |||
| // | |||
| // Microsoft dotNetMF Project | |||
| // Copyright �2004 Microsoft Corporation | |||
| // Copyright ©2004 Microsoft Corporation | |||
There was a problem hiding this comment.
@smaillet-ms I've replaced this with the correct copyright symbol. Saved the file with UTF-8 encoding and GitHub still shows a difference at this line.
If you really think this deserves the effort let me the encoding for the original code page and I'll try to revert this particular change.
There was a problem hiding this comment.
@josesimoes The original encoding of the file is Windows 1252. (ISO 8859-1 might work too)
There was a problem hiding this comment.
@miloush just saved it again with that encoding and seems to have remove that diff. Thanks for pointing that. 👏
There was a problem hiding this comment.
Ugh! :face_palm: Sigh... Thanks for finding this. Unfortunately fixing the encoding on mass is likely to cause more problems than it will help. Given that the current plan on vNext is to start with a clean slate so we can also do code style clean ups as we essentially import code into a new branch we can define and normalize the encoding for each file type. (And ideally find a way to detect deviations before the get into the repo)
There was a problem hiding this comment.
Yeah, I was thinking with the vNext in mind whether there is still any tool in the chain that couldn't handle Unicode...
There was a problem hiding this comment.
Embedded Assemblers? C? C++? Who knows what sort of shortcuts some of the available compilers use. (I can't remember if C++ officially supports Unicode text or even if it makes any mention of encoding of source files in any particular form, EBCDIC anyone? 😁 )
There was a problem hiding this comment.
@smaillet-ms EBCDIC I eat that for breakfast, it's my daily job ....
|
This solves the need for #492 |
I've tested this on real hardware with the Discovery4 solution.
The binaries sizes are: Tinybooter.bin 41372B & ER_FLASH 304880B
I've successfully build the MCBSTM32F400 but can't test it on hardware.
As I'm not entirely confortable nor can test it, I have NOT changed the scatter files and start up assembly code for MDK, only for GCC. Appreciate if someone knowledgeable on that platform could step in and help with that.
Ready for the code review and the criticism. 👷 😓 😄