Skip to content
This repository has been archived by the owner on Jun 5, 2019. It is now read-only.

Remove custom heap #486

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Remove custom heap #486

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
82e276d
Remove custom heap
josesimoes Aug 5, 2016
8179460
Merge branch 'dev' of https://github.com/NETMF/netmf-interpreter into…
josesimoes Aug 8, 2016
b977a84
Revert changes with C11 flags and extra debug settings
josesimoes Aug 8, 2016
ecd0fa3
Repalced "unknown" char with (hopefully right) copyright char
josesimoes Aug 9, 2016
6a8107e
Save - again - with Windows 1252 encoding
josesimoes Aug 9, 2016
27f1ea7
Remove SimpleHeap stubs projects
josesimoes Nov 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Application/EraseDeployment/EraseDeployment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@

#include "tinyhal.h"

HAL_DECLARE_NULL_HEAP();

void ApplicationEntryPoint()
{
int nSects = 0;
Expand Down
14 changes: 2 additions & 12 deletions Application/MicroBooter/MicroBooter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// <No description>
//
// Microsoft dotNetMF Project
// Copyright �2004 Microsoft Corporation
// Copyright ©2004 Microsoft Corporation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link

@miloush miloush Aug 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@josesimoes The original encoding of the file is Windows 1252. (ISO 8859-1 might work too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@miloush just saved it again with that encoding and seems to have remove that diff. Thanks for pointing that. 👏

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link

Choose a reason for hiding this comment

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

Yeah, I was thinking with the vNext in mind whether there is still any tool in the chain that couldn't handle Unicode...

Copy link
Member

Choose a reason for hiding this comment

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

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? 😁 )

Copy link

Choose a reason for hiding this comment

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

@smaillet-ms EBCDIC I eat that for breakfast, it's my daily job ....

// One Microsoft Way, Redmond, Washington 98052-6399 U.S.A.
// All rights reserved.
// MICROSOFT CONFIDENTIAL
Expand All @@ -22,8 +22,6 @@ BOOL MemStreamSeekBlockAddress( BlockStorageStream &stream, UINT32 address );
static SREC_Handler g_SREC;
#endif

HAL_DECLARE_CUSTOM_HEAP( SimpleHeap_Allocate, SimpleHeap_Release, SimpleHeap_ReAllocate );

#pragma arm section zidata = "s_SystemStates"
static INT32 s_SystemStates[SYSTEM_STATE_TOTAL_STATES];
#pragma arm section zidata
Expand Down Expand Up @@ -265,7 +263,7 @@ static BOOL Memory_Read( UINT32 address, UINT32 length, BYTE* data )

if(s_ReadBuffer == NULL)
{
s_ReadBuffer = (UINT8*)private_malloc(readBufferSize);
s_ReadBuffer = (UINT8*)malloc(readBufferSize);

ASSERT(s_ReadBuffer != NULL);
if(s_ReadBuffer == NULL) return FALSE;
Expand Down Expand Up @@ -433,14 +431,6 @@ void BootEntryLoader()

Events_Initialize();

UINT8* BaseAddress;
UINT32 SizeInBytes;

HeapLocation( BaseAddress, SizeInBytes );

// Initialize custom heap with heap block returned from CustomHeapLocation
SimpleHeap_Initialize( BaseAddress, SizeInBytes );

// this is the place where interrupts are enabled after boot for the first time after boot
ENABLE_INTERRUPTS();

Expand Down
2 changes: 1 addition & 1 deletion Application/MicroBooter/SrecProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ BOOL SREC_Handler::Process( char c )

if(!m_Stream.IsXIP())
{
dstExec = (UINT32)private_malloc(m_ImageLength);
dstExec = (UINT32)malloc(m_ImageLength);

MemStreamSeekBlockAddress( m_Stream, m_ImageStart );

Expand Down
38 changes: 19 additions & 19 deletions Application/TinyBooter/Commands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ struct BitFieldManager
{
const BlockDeviceInfo* deviceInfo = m_blockDevice->GetDeviceInfo();

data = (BYTE*)private_malloc( m_region->BytesPerBlock );
data = (BYTE*)malloc( m_region->BytesPerBlock );

if(data != NULL)
{
Expand All @@ -99,7 +99,7 @@ struct BitFieldManager
ConfigurationSector *pCfg = (ConfigurationSector*)data;
memset( (void*)&pCfg->SignatureCheck[ 0 ], 0xFF, sizeof(pCfg->SignatureCheck) );
m_blockDevice->Write( m_cfgPhysicalAddress, m_region->BytesPerBlock,data, FALSE );
private_free(data);
free(data);
}
else
{
Expand Down Expand Up @@ -150,7 +150,7 @@ struct BitFieldManager
else
{
UINT32 length = m_region->BytesPerBlock;
BYTE* dataptr = (BYTE*)private_malloc(length);
BYTE* dataptr = (BYTE*)malloc(length);

if(dataptr != NULL)
{
Expand All @@ -174,7 +174,7 @@ struct BitFieldManager

// write back to sector, as we only change one bit from 0 to 1, no need to erase sector
m_blockDevice->Write( m_cfgPhysicalAddress, length, dataptr, FALSE );
private_free(dataptr);
free(dataptr);
}

}
Expand Down Expand Up @@ -227,7 +227,7 @@ struct BitFieldManager
UINT32 length = sizeof(ConfigurationSector);

memset( &m_skipCfgSectorCheck, 0xff, sizeof(m_skipCfgSectorCheck) );
data = (BYTE*)private_malloc(length);
data = (BYTE*)malloc(length);
stream.Device->Read( m_cfgPhysicalAddress, length, (BYTE *)data );
configSector = (ConfigurationSector*)data;
m_signatureCheck = NULL;
Expand All @@ -253,7 +253,7 @@ struct BitFieldManager
m_signatureCheck = &m_skipCfgSectorCheck;
}

private_free(data);
free(data);

}
else // XIP device
Expand Down Expand Up @@ -476,7 +476,7 @@ static bool AccessMemory( UINT32 location, UINT32 lengthInBytes, BYTE* buf, int
{
if (mode == AccessMemory_Check)
{
bufPtr = (BYTE*) private_malloc(NumOfBytes);
bufPtr = (BYTE*) malloc(NumOfBytes);
if(!bufPtr) return false;
}

Expand All @@ -486,7 +486,7 @@ static bool AccessMemory( UINT32 location, UINT32 lengthInBytes, BYTE* buf, int
{
if (mode == AccessMemory_Check)
{
private_free(bufPtr);
free(bufPtr);
}

break;
Expand All @@ -495,7 +495,7 @@ static bool AccessMemory( UINT32 location, UINT32 lengthInBytes, BYTE* buf, int
if (mode == AccessMemory_Check)
{
*(UINT32*)buf = SUPPORT_ComputeCRC( bufPtr, NumOfBytes, *(UINT32*)buf );
private_free(bufPtr);
free(bufPtr);
}
}
break;
Expand Down Expand Up @@ -525,18 +525,18 @@ static bool AccessMemory( UINT32 location, UINT32 lengthInBytes, BYTE* buf, int
{
if(g_ConfigBuffer != NULL)
{
private_free(g_ConfigBuffer);
free(g_ConfigBuffer);
}
g_ConfigBufferLength = 0;

// g_ConfigBuffer = (UINT8*)private_malloc(pRegion->BytesPerBlock);
// g_ConfigBuffer = (UINT8*)malloc(pRegion->BytesPerBlock);
// Just allocate the configuration Sector size, configuration block can be large and not necessary to have that buffer.
g_ConfigBuffer = (UINT8*)private_malloc(g_ConfigBufferTotalSize);
g_ConfigBuffer = (UINT8*)malloc(g_ConfigBufferTotalSize);

}
else if(g_ConfigBufferTotalSize < ( g_ConfigBufferLength + lengthInBytes))
{
UINT8* tmp = (UINT8*)private_malloc(g_ConfigBufferLength + lengthInBytes);
UINT8* tmp = (UINT8*)malloc(g_ConfigBufferLength + lengthInBytes);

if(tmp == NULL)
{
Expand All @@ -545,7 +545,7 @@ static bool AccessMemory( UINT32 location, UINT32 lengthInBytes, BYTE* buf, int

memcpy( tmp, g_ConfigBuffer, g_ConfigBufferLength );

private_free(g_ConfigBuffer);
free(g_ConfigBuffer);

g_ConfigBuffer = tmp;
}
Expand Down Expand Up @@ -754,7 +754,7 @@ bool Loader_Engine::SignedDataState::VerifySignature( UINT8* signature, UINT32 l
const BlockDeviceInfo* deviceInfo = m_pDevice->GetDeviceInfo();
if(!deviceInfo->Attribute.SupportsXIP)
{
signCheckedAddr = (BYTE*)private_malloc(m_dataLength);
signCheckedAddr = (BYTE*)malloc(m_dataLength);
if (signCheckedAddr == NULL)
{
EraseMemoryAndReset();
Expand All @@ -764,7 +764,7 @@ bool Loader_Engine::SignedDataState::VerifySignature( UINT8* signature, UINT32 l
if(!m_pDevice->Read( m_dataAddress, m_dataLength, signCheckedAddr ))
{
EraseMemoryAndReset();
private_free(signCheckedAddr);
free(signCheckedAddr);

return false;
}
Expand All @@ -791,7 +791,7 @@ bool Loader_Engine::SignedDataState::VerifySignature( UINT8* signature, UINT32 l

if(!deviceInfo->Attribute.SupportsXIP)
{
private_free(signCheckedAddr);
free(signCheckedAddr);
}

return fret;
Expand Down Expand Up @@ -1542,7 +1542,7 @@ bool Loader_Engine::Monitor_FlashSectorMap( WP_Message* msg )

if(cnt == 1)
{
pData = (struct Flash_Sector*)private_malloc(rangeCount * sizeof(struct Flash_Sector));
pData = (struct Flash_Sector*)malloc(rangeCount * sizeof(struct Flash_Sector));

if(pData == NULL)
{
Expand Down Expand Up @@ -1582,7 +1582,7 @@ bool Loader_Engine::Monitor_FlashSectorMap( WP_Message* msg )

ReplyToCommand(msg, true, false, (void*)pData, rangeCount * sizeof (struct Flash_Sector) );

private_free(pData);
free(pData);

return true;
}
Expand Down
4 changes: 2 additions & 2 deletions Application/TinyBooter/ConfigurationManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void ConfigurationSectorManager::WriteConfiguration( UINT32 writeOffset, BYTE *d
// Copy the whole block to a buffer, for NonXIP or need to erase block
if ((eraseWrite) || (!m_fSupportsXIP))
{
configurationInBytes =(BYTE*)private_malloc(writeLengthInBytes);
configurationInBytes =(BYTE*)malloc(writeLengthInBytes);

// load data to the local buffer.
if (configurationInBytes)
Expand All @@ -154,7 +154,7 @@ void ConfigurationSectorManager::WriteConfiguration( UINT32 writeOffset, BYTE *d
// rewrite from the start of block
m_device->Write( m_cfgPhysicalAddress, writeLengthInBytes, configurationInBytes, FALSE );

private_free(configurationInBytes);
free(configurationInBytes);


}
Expand Down
2 changes: 1 addition & 1 deletion Application/TinyBooter/CryptoInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ bool CryptoState::VerifySignature( UINT32 keyIndex )
}

// free RAM buffer
private_free(g_ConfigBuffer);
free(g_ConfigBuffer);
g_ConfigBuffer = NULL;

return fRet;
Expand Down
11 changes: 0 additions & 11 deletions Application/TinyBooter/TinyBooter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,11 @@ Loader_Engine g_eng;

//--//

HAL_DECLARE_CUSTOM_HEAP( SimpleHeap_Allocate, SimpleHeap_Release, SimpleHeap_ReAllocate );

//--//

void ApplicationEntryPoint()
{
INT32 timeout = 20000; // 20 second timeout
bool enterBootMode = false;

// crypto API needs to allocate memory. Initialize simple heap for it.
UINT8* BaseAddress;
UINT32 SizeInBytes;

HeapLocation ( BaseAddress, SizeInBytes );
SimpleHeap_Initialize( BaseAddress, SizeInBytes );

g_eng.Initialize( HalSystemConfig.DebuggerPorts[ 0 ] );

// internal reset and stop check
Expand Down
2 changes: 0 additions & 2 deletions Application/TinyBooter/TinyBooterDecompressor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ typedef unsigned char UINT8;

int LZ77_Decompress( UINT8* inBuf, int inSize, UINT8* outBuf, int outSize );

HAL_DECLARE_NULL_HEAP();

typedef void (*APP_ENTRY_POINT)();

extern "C"
Expand Down
2 changes: 0 additions & 2 deletions CLR/Core/CLR_RT_Memory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

//--//
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


void CLR_RT_Memory::Reset()
Expand Down
16 changes: 8 additions & 8 deletions CLR/Debugger/Debugger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ bool CLR_DBG_Debugger::Monitor_FlashSectorMap( WP_Message* msg, void* owner )

if(cnt == 1)
{
pData = (struct Flash_Sector*)private_malloc(rangeCount * sizeof(struct Flash_Sector));
pData = (struct Flash_Sector*)malloc(rangeCount * sizeof(struct Flash_Sector));

if(pData == NULL)
{
Expand Down Expand Up @@ -467,7 +467,7 @@ bool CLR_DBG_Debugger::Monitor_FlashSectorMap( WP_Message* msg, void* owner )

dbg->m_messaging->ReplyToCommand( msg, true, false, (void*)pData, rangeCount * sizeof (struct Flash_Sector) );

private_free(pData);
free(pData);
}

return true;
Expand Down Expand Up @@ -1210,7 +1210,7 @@ bool CLR_DBG_Debugger::Debugging_MFUpdate_AuthCommand( WP_Message* msg, void* ow
{
int cmdSize = respLen + offsetof(CLR_DBG_Commands::Debugging_MFUpdate_AuthCommand::Reply, m_response);

CLR_DBG_Commands::Debugging_MFUpdate_AuthCommand::Reply* pTmp = (CLR_DBG_Commands::Debugging_MFUpdate_AuthCommand::Reply*)private_malloc(cmdSize);
CLR_DBG_Commands::Debugging_MFUpdate_AuthCommand::Reply* pTmp = (CLR_DBG_Commands::Debugging_MFUpdate_AuthCommand::Reply*)malloc(cmdSize);

if(pTmp != NULL)
{
Expand All @@ -1223,7 +1223,7 @@ bool CLR_DBG_Debugger::Debugging_MFUpdate_AuthCommand( WP_Message* msg, void* ow
}
else
{
private_free(pTmp);
free(pTmp);
}
}
}
Expand All @@ -1233,7 +1233,7 @@ bool CLR_DBG_Debugger::Debugging_MFUpdate_AuthCommand( WP_Message* msg, void* ow

if(pReply != &reply)
{
private_free(pReply);
free(pReply);
}

return true;
Expand Down Expand Up @@ -1297,7 +1297,7 @@ bool CLR_DBG_Debugger::Debugging_MFUpdate_GetMissingPkts( WP_Message* msg, void*

if(MFUpdate_GetMissingPackets(cmd->m_updateHandle, &s_missingPkts[0], &int32Cnt))
{
pReply = (CLR_DBG_Commands::Debugging_MFUpdate_GetMissingPkts::Reply*)private_malloc(sizeBytes);
pReply = (CLR_DBG_Commands::Debugging_MFUpdate_GetMissingPkts::Reply*)malloc(sizeBytes);

if(pReply != NULL)
{
Expand All @@ -1318,7 +1318,7 @@ bool CLR_DBG_Debugger::Debugging_MFUpdate_GetMissingPkts( WP_Message* msg, void*

if(pReply != &reply)
{
private_free(pReply);
free(pReply);
}

return true;
Expand Down Expand Up @@ -1953,7 +1953,7 @@ bool CLR_DBG_Debugger::Debugging_Thread_Get( WP_Message* msg, void* owner )

if(!fFound)
{
pThread = (CLR_RT_HeapBlock*)private_malloc(sizeof(CLR_RT_HeapBlock));
pThread = (CLR_RT_HeapBlock*)malloc(sizeof(CLR_RT_HeapBlock));

//Create the managed thread.
//This implies that there is no state in the managed object. This is not exactly true, as the managed thread
Expand Down
8 changes: 4 additions & 4 deletions CLR/Graphics/Graphics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ HRESULT CLR_GFX_Bitmap::CreateInstance( CLR_RT_HeapBlock& ref, const CLR_GFX_Bit
}
else
{
//The bitmap is too big to fit on the managed heap, so put it on the SimpleHeap
bitmap = (CLR_GFX_Bitmap*)SimpleHeap_Allocate(size);
//The bitmap is too big to fit on the managed heap, so put it on the heap
bitmap = (CLR_GFX_Bitmap*)malloc(size);

ref.SetInteger((CLR_UINT32)bitmap);
ref.PerformBoxingIfNeeded();
Expand Down Expand Up @@ -195,7 +195,7 @@ HRESULT CLR_GFX_Bitmap::CreateInstance( CLR_RT_HeapBlock& ref, const CLR_UINT8*
/* When loading a Windows BMP, GIF, or JPEG file, it is converted in-place to the native BPP.
* When loading a compressed TinyCLR Bitmap from a resource file, two bitmaps are needed to decompress, then convert.
* This fragments the heap and wastes space until the next garbage collection is done.
* When using the SimpleHeap, there is no relocation, so decompressing into a temp bitmap into the simpleheap wastes
* When using the heap, there is no relocation, so decompressing into a temp bitmap into the heap wastes
Copy link
Member

Choose a reason for hiding this comment

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

The comment shouldn't just blindly replace words with a GREP, it needs to be re-worded to reflect the consequences. The platform creator should still have the option of creating a simple heap/big object storage to support large bitmaps that are otherwise not supported in the managed heap. (One of the implementation details in the managed heap is an assumption on the maximum size of any single allocation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not following you here... A platform creator can make the standard heap as large as he wants to hold these graph and whatever stuff it's needed. I don't see why this comment is misleading or not providing enough explanations for an informed decision.

Copy link
Member

Choose a reason for hiding this comment

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

The large object heap was isolated for a reason, to allow the OEM to partition out memory used for LARGE objects. Throwing everything into one big heap may be a simple solution to port but can lead to fragmentation problems, which is why the large buffer and bitmap images were isolated into a separate heap. This might be something an OEM could manage using a custom implementation of malloc()/free() (e.g. use the size parameter of malloc to determine which underlying region to allocate from), but doing so is more difficult as replacing the standard functions is slightly less than intuitively obvious to the casual observer 😉 and compiler/tool specific. While I'm all for getting rid of custom heap use for the normal PAL/HAL work (that should stick with standard malloc for greater portability and re-use) I think it makes life easier for ports needing large buffers to still have this as an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My suggestion was to remove the custom heap. Which I did, for GCC. And I suspect it should be easy for someone knowledgeable to port this to MDK.

Wasn't aware of this nuance of "large objects". Feel free to take what's done and incorporate any changes required to making the options you mention available.

* memory 6.25% the size of the 16bpp bitmap that's saved.
*/

Expand Down Expand Up @@ -310,7 +310,7 @@ HRESULT CLR_GFX_Bitmap::DeleteInstance( CLR_RT_HeapBlock& ref )
if (blob->IsBoxed() && blob[ 1 ].DataType() == DATATYPE_U4)
{
bitmap = (CLR_GFX_Bitmap*)(blob[ 1 ].NumericByRefConst().u4);
SimpleHeap_Release (bitmap);
free(bitmap);
}
else
{
Expand Down
Loading