Game Development Community

TGEA 1.8 beta[Matt, must be careful code quality

by Henrysan · in Torque Game Engine Advanced · 12/07/2008 (7:35 am) · 9 replies

Hey, Matt

(English is not my mother language)
(I post that want somebody attach importance to code quality, not only for memory leak)
(these leak report I don't check carefully, just catch some involantary)

First congratulate, great work!

If we want use torque to make a BIG product, code quality is soooooo important!
See memory leak:
[
---------- Block 822 at 0x06271040: 52 bytes ----------
Call Stack:
c:\torque\tgea_1_8_beta_1\engine\source\app\badwordfilter.cpp (18): BadWordFilter::BadWordFilter
c:\torque\tgea_1_8_beta_1\engine\source\app\badwordfilter.cpp (31): BadWordFilter::create
c:\torque\tgea_1_8_beta_1\engine\source\app\mainloop.cpp (178): StandardMainLoop::init
c:\torque\tgea_1_8_beta_1\gameexamples\t3d\source\main.cpp (28): TorqueMain
c:\torque\tgea_1_8_beta_1\engine\source\platformwin32\winwindow.cpp (279): run
c:\torque\tgea_1_8_beta_1\engine\source\platformwin32\winwindow.cpp (343): WinMain
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (315): __tmainCRTStartup
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (187): WinMainCRTStartup
0x7C816D4F (File and line number not available): RegisterWaitForInputIdle

---------- Block 139 at 0x02F75798: 76 bytes ----------
Call Stack:
c:\torque\tgea_1_8_beta_1\engine\source\lightingsystem\basiclighting\basiclightmanager.cpp (74): BasicLightManager::createBasicLightInfo
c:\torque\tgea_1_8_beta_1\engine\source\lightingsystem\basiclighting\basiclightmanager.cpp (30): BasicLightManager::BasicLightManager
c:\torque\tgea_1_8_beta_1\engine\source\lightingsystem\basiclighting\basiclightmanager.h (54): BasicLightManager::get
c:\torque\tgea_1_8_beta_1\engine\source\lightingsystem\basiclighting\basicsceneobjectlightingplugin.cpp (115): BasicSceneObjectPluginFactory::BasicSceneObjectPluginFactory
c:\torque\tgea_1_8_beta_1\engine\source\lightingsystem\basiclighting\basicsceneobjectlightingplugin.cpp (111): 'dynamic initializer for 'p_BasicSceneObjectPluginFactory''
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (855): _initterm
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (293): _cinit

---------- Block 63 at 0x02F73070: 120 bytes ----------
Call Stack:
c:\torque\tgea_1_8_beta_1\engine\source\core\util\str.cpp (410): StrMem::get
c:\torque\tgea_1_8_beta_1\engine\source\core\util\str.cpp (561): SubString::operator new
c:\torque\tgea_1_8_beta_1\engine\source\core\util\str.cpp (643): String::String
c:\torque\tgea_1_8_beta_1\engine\source\clipmap\clipmap.cpp (23): 'dynamic initializer for 'ClipMapTextureProfile''
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (855): _initterm
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (293): _cinit
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (301): __tmainCRTStartup
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (187): WinMainCRTStartup
0x7C816D4F (File and line number not available): RegisterWaitForInputIdle

---------- Block 195 at 0x03E3DFB0: 5120 bytes ----------
Call Stack:
c:\torque\tgea_1_8_beta_1\engine\source\scenegraph\sceneobject.cpp (813): Container::Container
c:\torque\tgea_1_8_beta_1\engine\source\scenegraph\sceneobject.cpp (797): 'dynamic initializer for 'gClientContainer''
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (855): _initterm
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0dat.c (293): _cinit
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (301): __tmainCRTStartup
f:\rtm\vctools\crt_bld\self_x86\crt\src\crt0.c (187): WinMainCRTStartup
...................
...................
...................
]
and more...................

Also we refer other bug reports, but no people to fix it.
Yes, all these bug we can fix by self. But if the TGEA1.8 next week release, should we do combination
again? fix again?
Maybe we can check out CVS??????
If we can check out CVS, I think that we can do much help for TGEA/TGE.Please!!!

For BIG game product, must have high code quality!!!Not just see good GFX.(At this point, if the product
have a server, server must have high code quality!!)

#1
12/07/2008 (9:10 am)
Please don't just run some memory debugger on Torque and assume every report it generates indicates some programming error in Torque. All "leaks" indicated in your post are global data that get constructed when Torque starts up and will never get out of scope except when Torque exits. Thus, except if you were turning Torque into a DLL and wanted to have it unloadable, Torque will never actually leak memory here.

Disclaimer: This is not to say that there potentially aren't memory leaks in Torque.

And yes... I went through some of the dtors and made them clean up properly, global or not.
#2
12/07/2008 (11:40 am)
@Rene
Please check the code first!
In 1.7.0 beta there is more!!!
Yes I don't check 1.8 very carefully, just catch a report, but can you say that TGEA1.8 don't leak 1 byte???
I post this just for a example(these leak report I don't check carefully, just catch some involantary), want somebody attach importance to code quality, not just for memory leak! All we want code safe!
Hey Guy, be careful! :)

Eligible programmer don't say that "OS will reclaim memery so don't care that."
(even only new one object, not grow any more)

For example:
void BadWordFilter::create() {...}
void BadWordFilter::destroy(){} // why write it, but don't call it.

and also

void BasicLightManager::cleanup(){} // why write it, but don't call it.

I have no time to check it any more, I said again "All we want code safe, not just for memory leak."
that TGEA will better.
#3
12/07/2008 (12:00 pm)
Hmm, you were offended by my post. Sorry.

As stated in my disclaimer above, I did not claim that Torque may not have memory leaks, but all the relevant code in the above reports are *not* leaks. This is global data. It gets allocated on startup by globally instantiated data structures and then never freed which thus will cause it to show up in memory debuggers as leaking memory.

Example: each use of GFX_ImplementTextureProfile (such as those clipmap texture profiles above) will cause a String to be allocated that will never be freed. But that isn't a leak since these things are global and thus persist until program termination.

And yes, I checked the code before posting.
#4
12/07/2008 (12:08 pm)
@Rene
:)
I say again, "these leak report I don't check carefully, just catch some involantary."

and by the way,
You *new* object put to a global/class static/namespace pointer(even only have one object, not grow any more), Eligible programmer don't say that "OS will reclaim memery so don't care that."

GFX_ImplementTextureProfile-->StrMem(also HashTable smStrHash8, smStrHash16, StrMem::kill()): 'not local' variable depend on different compile unit, clear design???
#5
12/07/2008 (12:10 pm)
Hmm, think we disagree on that point. When a process terminates, all its private resources get freed by the system so why bother so any time spend on manually freeing each piece of allocated data is basically wasted time. It only matters, if you run as a DLL and want to be unloadable.

Can't follow your BadWorldFilter example, too. BadWorldFilter::destroy() properly cleans up.
#6
12/07/2008 (12:18 pm)
@Rene:

"BadWorldFilter::destroy() properly cleans up"???
I download the newest TGEA1.8 beat, your code? Can you catch the breakpoint?
:P~~~
#7
12/08/2008 (7:48 am)
I apologize for my wording, Henrysan. Have been kind of in an aggressive mood yesterday. Should have stayed off the boards :) I stand by *what* I have said, but *how* I said it was inappropriate and unkind.
#8
12/08/2008 (12:22 pm)
I would like to add that getting TGEA 1.7 to run in a browser (via plugin / dll) has been a huge bear because of these kind of "OS will clean it up" assumptions.
#9
12/08/2008 (1:13 pm)
Quote:It only matters, if you run as a DLL and want to be unloadable.

Doesn't InstantAction do this? (edit maybe IA doesn't use TGEA, but other web browser plugins might)

Seems to me that this should be fixed and not discarded.

Good catch, Henrysan.