Re: [Warzone-dev] [patch] MALLOC/FREE issues
Christian Vest Hansen schreef: 2007/1/1, Giel van Schijndel [EMAIL PROTECTED]: Per Inge Mathisen schreef: On 1/1/07, Giel van Schijndel [EMAIL PROTECTED] wrote: A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Interesting. Very interesting. How about setting up a branch with this new memory system, and then merge it back once it feels a little less shakey? I'de sure like to see this performance improvement with my own eyes. I like the idea of a separate branch for working on the memory system. Sounds like a good place as a testing ground for every thing that involves memory management. -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
On 1/1/07, Giel van Schijndel [EMAIL PROTECTED] wrote: When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? All memory allocations now take place using ordinary malloc/free, instead of inside fixed size block heaps, yes. A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Interesting. For every time when I now encounter enemy units approaching me over a hill (or some other terrain that requires height checking for visibility) an assert gets fired from src/visibility.c:164 (in function: SDWORD visObjHeight(BASE_OBJECT *psObject)). It seems like the memory which contains psObject now somehow gets corrupted which causes psObject-type to be overwritten. I'm guessing this is now being exposed because the previous malloc allocated 32 bytes extra before and after the requested memspace as a safety zone. PS It only seems to occur when I pass the --cheat parameter to WZ Uploaded a savegame and my own compilation (only diff to r591 is your mempatch) that cause the assert-loop here: http://wz-debug.mortis.eu/ I cannot reproduce it with the savegame. I see some odd artefacts, though - the ocean tiles do not tile well, and some error: ... warning ... messages, but no asserts. I do not have time to test this in any depth. If you can get me a backtrace, I could look at that. The next step is probably to look at the valgrind report and check those invalid reads and conditional jumps. Those need to be fixed (I think most of them are present in current svn also? perhaps a good idea would be to fix them in svn current first, then see if we get any _new_ ones after applying the patch). - Per ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Per Inge Mathisen schreef: On 1/1/07, Giel van Schijndel [EMAIL PROTECTED] wrote: When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? All memory allocations now take place using ordinary malloc/free, instead of inside fixed size block heaps, yes. A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Interesting. For every time when I now encounter enemy units approaching me over a hill (or some other terrain that requires height checking for visibility) an assert gets fired from src/visibility.c:164 (in function: SDWORD visObjHeight(BASE_OBJECT *psObject)). It seems like the memory which contains psObject now somehow gets corrupted which causes psObject-type to be overwritten. I'm guessing this is now being exposed because the previous malloc allocated 32 bytes extra before and after the requested memspace as a safety zone. PS It only seems to occur when I pass the --cheat parameter to WZ Uploaded a savegame and my own compilation (only diff to r591 is your mempatch) that cause the assert-loop here: http://wz-debug.mortis.eu/ I cannot reproduce it with the savegame. I see some odd artefacts, though - the ocean tiles do not tile well, and some error: ... warning ... messages, but no asserts. I do not have time to test this in any depth. If you can get me a backtrace, I could look at that. The next step is probably to look at the valgrind report and check those invalid reads and conditional jumps. Those need to be fixed (I think most of them are present in current svn also? perhaps a good idea would be to fix them in svn current first, then see if we get any _new_ ones after applying the patch). As requested a backtrace: (gdb) run Starting program: C:\Program Files\Warzone 2100/warzone2100.exe --cheat --datadi r C:/Program Files/Warzone 2100 --savegame Little Egypt crash on visibility H eight.gam Program received signal SIGTRAP, Trace/breakpoint trap. 0x77c05b62 in OpenAL32_dll_iname () (gdb) bt #0 0x77c05b62 in OpenAL32_dll_iname () #1 0x0001 in ?? () #2 0x004d4b8e in _Z12visObjHeightP12_base_object (psObject=0x4a1d010) at visibility.c:164 #3 0x004d59c3 in _Z13visibleObjectP12_base_objectS0_ (psViewer=0x4a15208, psTarget=0x4a1d010) at visibility.c:646 #4 0x004086dd in _Z17actionUpdateDroidP6_droid (psDroid=0x4a15208) at action.c:1430 #5 0x00427ce3 in _Z11droidUpdateP6_droid (psDroid=0x4a15208) at droid.c:1331 #6 0x0046b64e in _Z8gameLoopv () at loop.c:320 #7 0x005038ad in SDL_main (argc=6, argv=0x22fce0) at main.c:683 #8 0x005dc2eb in console_main (argc=6, argv=0x22fce0) at ./src/main/win32/SDL_win32_main.c:217 #9 0x005dc4a2 in [EMAIL PROTECTED] (hInst=0x40, hPrev=0x0, szCmdLine=0x241f10 --cheat --datadir \C:/Program Files/Warzone 2100\ --savegame \Little Egypt crash on visibility Height.gam\, sw=10) at ./src/main/win32/SDL_win32_main.c:353 #10 0x0057fbaa in main () (gdb) PS it seems also only to occur when I order the units in group 1 to move leftwards (up that tiny hill). -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Per Inge Mathisen schreef: Updated memory cleanup patch. It is basically the same as the one posted last year. I could not find any problems with it from a very basic feature testing (starting campaign, multi, tutorial, loading and saving), but running it a bit in valgrind reports a huge amount of lost memory. I do not know if this is because the patch introduces memory loss or because valgrind can now detect existing memory lost, though. The patch is in two steps, apply to test as you feel like it: STEP 1 - use malloc and free instead of custom MALLOC and FREE ... STEP 2 - rip out block memory system entirely ... If you apply these patches to a clean repository it won't compile because some source files still depend on the MALLOC and FREE macros. -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
On 12/31/06, Giel van Schijndel [EMAIL PROTECTED] wrote: If you apply these patches to a clean repository it won't compile because some source files still depend on the MALLOC and FREE macros. The MALLOC and FREE macros are still there, in mem.h, just defined differently. Did you try it? - Per ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Per Inge Mathisen schreef: On 12/31/06, Giel van Schijndel [EMAIL PROTECTED] wrote: If you apply these patches to a clean repository it won't compile because some source files still depend on the MALLOC and FREE macros. The MALLOC and FREE macros are still there, in mem.h, just defined differently. Did you try it? Ah, just noticed I've just trashed the entire mem.h file. Compiles fine now. When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? Wen just scanning the diffs the modifications look quite nice to me. All runtime that I've already had with the produced executable has had no problems (in fact it had a few less problems). A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Giel van Schijndel schreef: Per Inge Mathisen schreef: On 12/31/06, Giel van Schijndel [EMAIL PROTECTED] wrote: If you apply these patches to a clean repository it won't compile because some source files still depend on the MALLOC and FREE macros. The MALLOC and FREE macros are still there, in mem.h, just defined differently. Did you try it? Ah, just noticed I've just trashed the entire mem.h file. Compiles fine now. When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? Wen just scanning the diffs the modifications look quite nice to me. All runtime that I've already had with the produced executable has had no problems (in fact it had a few less problems). A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Seems that when applying your patch either an existing bug gets exposed or a new one created. For every time when I now encounter enemy units approaching me over a hill (or some other terrain that requires height checking for visibility) an assert gets fired from src/visibility.c:164 (in function: SDWORD visObjHeight(BASE_OBJECT *psObject)). It seems like the memory which contains psObject now somehow gets corrupted which causes psObject-type to be overwritten. I'm guessing this is now being exposed because the previous malloc allocated 32 bytes extra before and after the requested memspace as a safety zone. -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Giel van Schijndel schreef: Giel van Schijndel schreef: Per Inge Mathisen schreef: On 12/31/06, Giel van Schijndel [EMAIL PROTECTED] wrote: If you apply these patches to a clean repository it won't compile because some source files still depend on the MALLOC and FREE macros. The MALLOC and FREE macros are still there, in mem.h, just defined differently. Did you try it? Ah, just noticed I've just trashed the entire mem.h file. Compiles fine now. When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? Wen just scanning the diffs the modifications look quite nice to me. All runtime that I've already had with the produced executable has had no problems (in fact it had a few less problems). A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Seems that when applying your patch either an existing bug gets exposed or a new one created. For every time when I now encounter enemy units approaching me over a hill (or some other terrain that requires height checking for visibility) an assert gets fired from src/visibility.c:164 (in function: SDWORD visObjHeight(BASE_OBJECT *psObject)). It seems like the memory which contains psObject now somehow gets corrupted which causes psObject-type to be overwritten. I'm guessing this is now being exposed because the previous malloc allocated 32 bytes extra before and after the requested memspace as a safety zone. PS It only seems to occur when I pass the --cheat parameter to WZ -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Giel van Schijndel schreef: Giel van Schijndel schreef: Giel van Schijndel schreef: When looking at the code I'm guessing that all you've changed is the removal of all direct calls to the blockheap system throughout all src/ files? Wen just scanning the diffs the modifications look quite nice to me. All runtime that I've already had with the produced executable has had no problems (in fact it had a few less problems). A huge difference I'm noticing is that now my FPS actually comes above 25 on my laptop. Before using your patch I'd only have an FPS between 18-25 (yes the FPS limit was set on 60 as well then), now it runs on 60 while 60 currently is my set limit. Seems that when applying your patch either an existing bug gets exposed or a new one created. For every time when I now encounter enemy units approaching me over a hill (or some other terrain that requires height checking for visibility) an assert gets fired from src/visibility.c:164 (in function: SDWORD visObjHeight(BASE_OBJECT *psObject)). It seems like the memory which contains psObject now somehow gets corrupted which causes psObject-type to be overwritten. I'm guessing this is now being exposed because the previous malloc allocated 32 bytes extra before and after the requested memspace as a safety zone. PS It only seems to occur when I pass the --cheat parameter to WZ Uploaded a savegame and my own compilation (only diff to r591 is your mempatch) that cause the assert-loop here: http://wz-debug.mortis.eu/ -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Sorry, I've been offline for a week. I thought my memory cleanup patch had been permanently lost in my harddisk crash a while ago. Good thing I have a habit of sending patches to the list frequently, even though they don't really work quite yet ;-) The problems I encountered were in the scripting engine, which I did not take time to understand, so if Troman wants to look into making it work there, I can see if I could update it again. Memory pools, properly done, have some very interesting properties, and I know they are put do some good use in the Apache webserver and in Subversion (both using the APR library). I doubt Warzone uses memory pools for anything very interesting. From what I could tell it was doing very rudimentary memory corruption checking, which is way better done with valgrind, and I do not think the memory pool system in Warzone is not cooperating well with valgrind. - Per ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
- Original Message - From: Per Inge Mathisen [EMAIL PROTECTED] To: Development list warzone-dev@gna.org Sent: Saturday, December 30, 2006 5:38 PM Subject: Re: [Warzone-dev] [patch] MALLOC/FREE issues Sorry, I've been offline for a week. I thought my memory cleanup patch had been permanently lost in my harddisk crash a while ago. Good thing I have a habit of sending patches to the list frequently, even though they don't really work quite yet ;-) The problems I encountered were in the scripting engine, which I did not take time to understand, so if Troman wants to look into making it work there, I can see if I could update it again. Do you remember what exactly caused problems? Memory pools, properly done, have some very interesting properties, and I know they are put do some good use in the Apache webserver and in Subversion (both using the APR library). I doubt Warzone uses memory pools for anything very interesting. From what I could tell it was doing very rudimentary memory corruption checking, which is way better done with valgrind, and I do not think the memory pool system in Warzone is not cooperating well with valgrind. - Per ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Giel van Schijndel schreef:
Giel van Schijndel schreef:
Christian Ohm schreef:
On Thursday, 28 December 2006 at 20:06, Dennis Schridde wrote:
I think not too long ago Per said something like simply replacing MALLOC
was
not possible... Dunno if I remember that correctly or how his patch fixed
that...
Just defining MALLOC/FREE to malloc/free etc. (see below) compiles a
binary that runs at least the tutorial for a short while (haven't done
more testing). So I guess we could just add those defines
unconditionally, and if nothing breaks for a while, remove them from the
code itself.
--- lib/framework/mem.h (revision 579)
+++ lib/framework/mem.h (working copy)
@@ -78,5 +78,10 @@
#endif // DEBUG_MALLOC
+#define MALLOC(size) malloc(size)
+#define FREE(ptr) free(ptr)
+#define PTRVALID(ptr, size) (TRUE)
+#define MEMORYREPORT(file)
+#define MEMORYTREAP(file)
#endif
While I'm sure that will work correctly in any release build I'm not so
sure it will in the debug build.
That's because the debug build assigns a struct around the malloc'ed
memory for memory tracing.
Also you might want to use a bit different define for FREE, the current
codebase _might_ depend upon it, to prevent accessing freed memory:
#define FREE(ptr)do { free(ptr); ptr = NULL; } while(0)
Ok, pretend I didn't say using malloc() and free() like that would work
fine. Because it won't, not as long as the blockheap memory system is
being used. This system turned on by memSetBlockHeap(!NULL) and off by
memSetBlockHeap(NULL), so before you can use plain malloc() and free()
you'll have to remove all calls to memSetBlockHeap() (blockheap system
is turned off on initialization of the memory system).
Well, I've done some cleaning up in the memory system, result is the
attached patch.
This patch does the following:
* remove lots of code that never gets executed (because of if(0),
#if(0), or by using other condition checking which always sees a 0 as
condition through some kind of function or variable).
* remove code that doesn't actually do anything ( debug ( LOG_NEVER,
... ); included) (e.g. calculating and assigning values to a variable
which then never gets referred again)
* remove code that *does* do something but is completely useless (e.g.
setting all memory to 0x00 before passing it to free() )
* remove a whole lot of (unused/uncalled) debugging functions that do
nothing more than outputting/printing stats about the current state of
the memory allocations/usage
* remove (or comment out) code that encourages buggy programming (e.g.
assuming all acquired memory is initialized)
* remove some preprocessor macros which don't ever get used (yes, I
searched/scanned every single source file in the trunk)
* turned some ASSERT (FALSE, ...) statements into the condition they
are actually guarding rather than depending on the programmer to go and
look for the if(...) condition right above this ASSERT
* fixed (two I believe) some compiler warnings
* inserted some comments to clarify what some very odd looking
non-intuitive code does
* moved variable declaration more down into code where it is
initialized (i.e. applying RAII there where it easy to do so)
* fixed some minor bugs (e.g. triggering two ASSERTS, while the first
one was well enough to predict the second one would occur as well)
If I've done everything correct, then all this patch should do is
cleanup code _almost_ (i.e. some double triggered asserts will be gone,
and some bugs because of lacking RAII will now not occur) without
altering the program's behaviour.
--
Giel
Index: lib/framework/block.c
===
--- lib/framework/block.c (revision 579)
+++ lib/framework/block.c (working copy)
@@ -131,14 +131,6 @@
{
BLOCK_HEAP_MEM *psCurr, *psNext;
-#ifdef DEBUG_BLOCK
- if (psHeap-psMemTreap != NULL)
- {
- debug( LOG_NEVER, blkDestroy: %s at %d: memory allocated :\n,
psHeap-pFileName, psHeap-line );
- memRecReport(psHeap-psMemTreap);
- }
-#endif
-
LIST_REMOVE(psBlockList, psHeap, BLOCK_HEAP);
for(psCurr=psHeap-psBlocks; psCurr; psCurr=psNext)
@@ -150,97 +142,6 @@
RFREE(psHeap);
}
-void memMemoryDump(MEM_NODE *Node);
-
-
-void blkPrintDetails(BLOCK_HEAP *psHeap)
-{
- if (psHeap!=NULL)
- {
-
-#ifdef DEBUG_BLOCK
- UDWORD Left =
(UDWORD)((psHeap-psBlocks-pMem)+(psHeap-psBlocks-size)-(psHeap-psBlocks-pFree));
- debug( LOG_NEVER, ptr=%p init=%d ext=%d used=%d (Start=$%p
Free=$%p Left=%d)\n, psHeap,psHeap-init, psHeap-ext,psHeap-TotalAllocated,
psHeap-psBlocks-pMem, psHeap-psBlocks-pFree, Left );
- memMemoryDump(psHeap-psMemTreap);
-#else
- debug( LOG_NEVER, ptr=%p init=%d ext=%d\n, psHeap,
psHeap-init, psHeap-ext );
-#endif
-
- }
- else
- {
- debug(
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Am Freitag, 29. Dezember 2006 16:12 schrieb Giel van Schijndel: Well, I've done some cleaning up in the memory system, result is the attached patch. This patch does the following: * remove lots of code that never gets executed (because of if(0), #if(0), or by using other condition checking which always sees a 0 as condition through some kind of function or variable). * remove code that doesn't actually do anything ( debug ( LOG_NEVER, ... ); included) (e.g. calculating and assigning values to a variable which then never gets referred again) * remove code that *does* do something but is completely useless (e.g. setting all memory to 0x00 before passing it to free() ) * remove a whole lot of (unused/uncalled) debugging functions that do nothing more than outputting/printing stats about the current state of the memory allocations/usage * remove (or comment out) code that encourages buggy programming (e.g. assuming all acquired memory is initialized) * remove some preprocessor macros which don't ever get used (yes, I searched/scanned every single source file in the trunk) * turned some ASSERT (FALSE, ...) statements into the condition they are actually guarding rather than depending on the programmer to go and look for the if(...) condition right above this ASSERT * fixed (two I believe) some compiler warnings * inserted some comments to clarify what some very odd looking non-intuitive code does * moved variable declaration more down into code where it is initialized (i.e. applying RAII there where it easy to do so) * fixed some minor bugs (e.g. triggering two ASSERTS, while the first one was well enough to predict the second one would occur as well) If I've done everything correct, then all this patch should do is cleanup code _almost_ (i.e. some double triggered asserts will be gone, and some bugs because of lacking RAII will now not occur) without altering the program's behaviour. Are you sure the removal of memMemoryReport() from memShutDown() doesn't change anything? I remember reading some memory allocation stats on game close... There are also some LOG_NEVERs which don't seem so unused... never: blkReset: ../src/init.c at 1024: memory allocated : never: blkReset: ../src/init.c at 1024: memory usage: never: Blocks allocated 24525k, Memory allocated 4484k memory: memMemoryReport: No memory allocated --Dennis pgp1itoUXhSc6.pgp Description: PGP signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Dennis Schridde schreef:
Am Freitag, 29. Dezember 2006 16:12 schrieb Giel van Schijndel:
Well, I've done some cleaning up in the memory system, result is the
attached patch.
This patch does the following:
* remove lots of code that never gets executed (because of if(0),
#if(0), or by using other condition checking which always sees a 0 as
condition through some kind of function or variable).
* remove code that doesn't actually do anything ( debug ( LOG_NEVER,
... ); included) (e.g. calculating and assigning values to a variable
which then never gets referred again)
* remove code that *does* do something but is completely useless (e.g.
setting all memory to 0x00 before passing it to free() )
* remove a whole lot of (unused/uncalled) debugging functions that do
nothing more than outputting/printing stats about the current state of
the memory allocations/usage
* remove (or comment out) code that encourages buggy programming (e.g.
assuming all acquired memory is initialized)
* remove some preprocessor macros which don't ever get used (yes, I
searched/scanned every single source file in the trunk)
* turned some ASSERT (FALSE, ...) statements into the condition they
are actually guarding rather than depending on the programmer to go and
look for the if(...) condition right above this ASSERT
* fixed (two I believe) some compiler warnings
* inserted some comments to clarify what some very odd looking
non-intuitive code does
* moved variable declaration more down into code where it is
initialized (i.e. applying RAII there where it easy to do so)
* fixed some minor bugs (e.g. triggering two ASSERTS, while the first
one was well enough to predict the second one would occur as well)
If I've done everything correct, then all this patch should do is
cleanup code _almost_ (i.e. some double triggered asserts will be gone,
and some bugs because of lacking RAII will now not occur) without
altering the program's behaviour.
Are you sure the removal of memMemoryReport() from memShutDown() doesn't
change anything? I remember reading some memory allocation stats on game
close...
There are also some LOG_NEVERs which don't seem so unused...
never: blkReset: ../src/init.c at 1024: memory allocated :
never: blkReset: ../src/init.c at 1024: memory usage:
never: Blocks allocated 24525k, Memory allocated 4484k
memory: memMemoryReport: No memory allocated
The output of memMemoryReport depends upon the definition of
REALLY_DEBUG_MALLOC. Which is definednowhere throughout the source, in
fact it is only undefined once in lib/framework/mem.h:20 without being
defined before.
If however it where to be defined then memRecReport (also found in
mem.c) actually does something (otherwise it only returns 0). When
REALLY_DEBUG_MALLOC isn't defined however, all memMemoryReport does is
either:
debug(LOG_MEMORY, memMemoryReport: No memory allocated); or
debug(LOG_MEMORY, memMemoryReport: Total memory allocated is %d bytes,
TotMem); where TotMem = 0 so it really is this:
debug(LOG_MEMORY, memMemoryReport: Total memory allocated is 0 bytes);
When REALLY_DEBUG_MALLOC is defined however, memRecReport prints out (by
means of debug( LOG_MEMORY, ... )) a bunch of allocated memory chunks
along with the file and line where it got requested plus the requested
size. Then finally it also prints out the total used size.
Conclusion: when REALLY_DEBUG_MALLOC is not defined then memMemoryReport
is of no value, if however it is defined it might be useful in tracking
memory usage.
So I've adjusted my patch to only add some `#ifdef
REALLY_DEBUG_MALLOC'-statements around the code that uses memMemoryReport.
Then in for your info an explanation why in block.c there are three
functions removed: blkPrintDetails, blkReport and blkSpecialFree.
Except for blkPrintDetails none of them ever get called, and
blkPrintDetails only gets called by blkReport.
--
Giel
Index: lib/framework/block.c
===
--- lib/framework/block.c (revision 581)
+++ lib/framework/block.c (working copy)
@@ -135,7 +135,9 @@
if (psHeap-psMemTreap != NULL)
{
debug( LOG_NEVER, blkDestroy: %s at %d: memory allocated :\n,
psHeap-pFileName, psHeap-line );
+#ifdef REALLY_DEBUG_MALLOC
memRecReport(psHeap-psMemTreap);
+#endif
}
#endif
@@ -150,97 +152,6 @@
RFREE(psHeap);
}
-void memMemoryDump(MEM_NODE *Node);
-
-
-void blkPrintDetails(BLOCK_HEAP *psHeap)
-{
- if (psHeap!=NULL)
- {
-
-#ifdef DEBUG_BLOCK
- UDWORD Left =
(UDWORD)((psHeap-psBlocks-pMem)+(psHeap-psBlocks-size)-(psHeap-psBlocks-pFree));
- debug( LOG_NEVER, ptr=%p init=%d ext=%d used=%d (Start=$%p
Free=$%p Left=%d)\n, psHeap,psHeap-init, psHeap-ext,psHeap-TotalAllocated,
psHeap-psBlocks-pMem, psHeap-psBlocks-pFree, Left );
- memMemoryDump(psHeap-psMemTreap);
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Am Freitag, 29. Dezember 2006 23:18 schrieb Giel van Schijndel: Dennis Schridde schreef: Am Freitag, 29. Dezember 2006 16:12 schrieb Giel van Schijndel: Well, I've done some cleaning up in the memory system, result is the attached patch. This patch does the following: * remove lots of code that never gets executed (because of if(0), #if(0), or by using other condition checking which always sees a 0 as condition through some kind of function or variable). * remove code that doesn't actually do anything ( debug ( LOG_NEVER, ... ); included) (e.g. calculating and assigning values to a variable which then never gets referred again) * remove code that *does* do something but is completely useless (e.g. setting all memory to 0x00 before passing it to free() ) * remove a whole lot of (unused/uncalled) debugging functions that do nothing more than outputting/printing stats about the current state of the memory allocations/usage * remove (or comment out) code that encourages buggy programming (e.g. assuming all acquired memory is initialized) * remove some preprocessor macros which don't ever get used (yes, I searched/scanned every single source file in the trunk) * turned some ASSERT (FALSE, ...) statements into the condition they are actually guarding rather than depending on the programmer to go and look for the if(...) condition right above this ASSERT * fixed (two I believe) some compiler warnings * inserted some comments to clarify what some very odd looking non-intuitive code does * moved variable declaration more down into code where it is initialized (i.e. applying RAII there where it easy to do so) * fixed some minor bugs (e.g. triggering two ASSERTS, while the first one was well enough to predict the second one would occur as well) If I've done everything correct, then all this patch should do is cleanup code _almost_ (i.e. some double triggered asserts will be gone, and some bugs because of lacking RAII will now not occur) without altering the program's behaviour. Are you sure the removal of memMemoryReport() from memShutDown() doesn't change anything? I remember reading some memory allocation stats on game close... There are also some LOG_NEVERs which don't seem so unused... never: blkReset: ../src/init.c at 1024: memory allocated : never: blkReset: ../src/init.c at 1024: memory usage: never: Blocks allocated 24525k, Memory allocated 4484k memory: memMemoryReport: No memory allocated If the memMemoryReport will ever only output No memory allocated, then ok. I wonder though how the other report is created... And it seems to have some value. If that is not true, this lines are generated by some other function (you did not remove) or they output only some invalid numbers, then I don't fear to remove them either. I just wondered why you seemed to remove something which seemed to have some sense. If it hasn't, it's ok, remove them... --Dennis pgpcDLIMbZgD0.pgp Description: PGP signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] [patch] MALLOC/FREE issues
Dennis Schridde schreef: Am Freitag, 29. Dezember 2006 23:18 schrieb Giel van Schijndel: Dennis Schridde schreef: Am Freitag, 29. Dezember 2006 16:12 schrieb Giel van Schijndel: Well, I've done some cleaning up in the memory system, result is the attached patch. This patch does the following: * remove lots of code that never gets executed (because of if(0), #if(0), or by using other condition checking which always sees a 0 as condition through some kind of function or variable). * remove code that doesn't actually do anything ( debug ( LOG_NEVER, ... ); included) (e.g. calculating and assigning values to a variable which then never gets referred again) * remove code that *does* do something but is completely useless (e.g. setting all memory to 0x00 before passing it to free() ) * remove a whole lot of (unused/uncalled) debugging functions that do nothing more than outputting/printing stats about the current state of the memory allocations/usage * remove (or comment out) code that encourages buggy programming (e.g. assuming all acquired memory is initialized) * remove some preprocessor macros which don't ever get used (yes, I searched/scanned every single source file in the trunk) * turned some ASSERT (FALSE, ...) statements into the condition they are actually guarding rather than depending on the programmer to go and look for the if(...) condition right above this ASSERT * fixed (two I believe) some compiler warnings * inserted some comments to clarify what some very odd looking non-intuitive code does * moved variable declaration more down into code where it is initialized (i.e. applying RAII there where it easy to do so) * fixed some minor bugs (e.g. triggering two ASSERTS, while the first one was well enough to predict the second one would occur as well) If I've done everything correct, then all this patch should do is cleanup code _almost_ (i.e. some double triggered asserts will be gone, and some bugs because of lacking RAII will now not occur) without altering the program's behaviour. Are you sure the removal of memMemoryReport() from memShutDown() doesn't change anything? I remember reading some memory allocation stats on game close... There are also some LOG_NEVERs which don't seem so unused... never: blkReset: ../src/init.c at 1024: memory allocated : never: blkReset: ../src/init.c at 1024: memory usage: never: Blocks allocated 24525k, Memory allocated 4484k memory: memMemoryReport: No memory allocated If the memMemoryReport will ever only output No memory allocated, then ok. I wonder though how the other report is created... And it seems to have some value. If that is not true, this lines are generated by some other function (you did not remove) or they output only some invalid numbers, then I don't fear to remove them either. I just wondered why you seemed to remove something which seemed to have some sense. If it hasn't, it's ok, remove them... If you really mean output generated like that which you've quoted above (i.e. only the never: ... lines) then that output is generated by blkReset (in block.c) which doesn't use memMemoryReport. It does use memRecReport though, but that one is only called (ignoring its return value) while it really doesn't print anything when REALLY_DEBUG_MALLOC isn't defined. -- Giel signature.asc Description: OpenPGP digital signature ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
