To Xavi and All,
>> Thank you Przemek. It's not so easy because these solutions don't
>> work with C++ new operator.
>
> If you want to include memory allocated by C++ code to FM static
> then it's necessary to overload new and delete operators and redirect
> them to hb_xgrab()/hb_xfree(). hbmk2 has an option to enable such
> overloading. Anyhow C preprocessor is too week to automatically insert
> instruction which can safe __FILE__ and __LINE__ values so it cannot
> be cleanly done. Using macros like HB_TRACEFM( x ) you send is not
> solution too because in C++ code NEW/DELETE operators can be executed
> indirectly. I.e.:
> {
> my_type x;
> f( &x );
> }
> can activate code constructor and destructors which executes indirectly
> some other code which can allocated new memory. To correctly set file
> and line information in such code you will have to make sth like:
> {
> HB_TRACEFM( my_type x ); // this code is illegal in ANSI C mode !!!
> HB_TRACEFM( f( &x ) );
> HB_TRACEFM( ) // dummy call to set valid line and file info for
> // destructors which also can allocate some memory
> }
>
> In fact for real C++ code using extensively constructors and destructors
> you will have to enclose nearly _each_ line of code inside HB_TRACEFM()
> so for me HB_TRACEFM() isn't reasonable solution, too.
>
> Please also note that:
> HB_TRACEFM( my_type x );
> just like:
> HB_TRACEFM( void * ptr = hb_xgrab( 100 ) );
...
> generates in hidden way code which is not ANSI C compatible what creates
> yet another problem.
>
> Instead of using such HB_TRACEFM() macro I suggest to simply define
> macro which will set file and line number for FM module, i.e.:
>
> #define HB_TR_SET_CREATE(l) do { \
> hb_tr_level_ = l; \
> hb_tr_file_ = __FILE__; \
> hb_tr_line_ = __LINE__; \
> } while( 0 )
>
> #if HB_TR_LEVEL >= HB_TR_DEBUG
> #define HB_TR_SET_HB_TR_DEBUG() HB_TR_SET_CREATE(HB_TR_DEBUG)
> #else
> #define HB_TR_SET_HB_TR_DEBUG()
> #endif
> [...]
> #if 1 /* always! */
> #define HB_TR_SET_HB_TR_ALWAYS() HB_TR_SET_CREATE(HB_TR_ALWAYS)
> #else
> #define HB_TR_SET_HB_TR_ALWAYS()
> #endif
>
> #define HB_TR_SET(l) HB_TR_SET_##l()
>
> so user can mark any place in his code before calling hb_xgrab()
> or activating any other code which may allocates new memory by simple
> adding HB_TR_SET( HB_TR_DEBUG ); i.e.
>
> HB_TR_SET( HB_TR_DEBUG ); // code marker, store file and line info
> ptr = hb_xgrab( 100 );
>
> In such version it's explicitly visible for users what such macros does
> and which lines will be reported by FM statistic module.
> And if you still prefer HB_TRACEFM() then you can simply define in your
> code:
> HB_TRACEFM( x ) HB_TR_SET( HB_TR_ALWAYS ); x
>
> In the code you created you are storing with each memory block also
> the code inside HB_TRACEFM() macro ( #x ) in the 264 bytes buffer
> attached to each memory block. For me it seems to be redundant. Having
> file name and line number is enough to check the exact code so it does
> not give any new functionality but increase the overhead created by
> FM statistic module reducing the speed and increasing total memory
> usage. I agree that final FM log looks nicer with such feature but
> personally I do not think that such reason is important enough. Anyhow
> this are only my personal feelings so it should be rather group
> decision. If other developers things that it's important to attach
> text with source code inside HB_TRACEFM() macro to each memory block
> allocated by FM statistic module to report such lines in fm.log file
> and agree to accept slowness and additional memory overhead because
> information about source file name and line number is not enough
> then we can add it.
>
>> Attache files ready for committed, I've taken care to preserve intact
>> the largest existing code as possible.
>> Please, you can review especially in MT mode. I've tried it with
>> success in a large multi-threaded application without conflict or
>> deadlock but maybe I missed something and I don't know force the
>> error.
>
> I see that you used TSD structure allocated on HVM stack to make it
> MT safe. But it creates one serious problem. It means that it's
> not possible to activate this code before HVM stack is created
> by application startup code and it's not possible to use hb_xgrab()
> by non HVM threads. I only guess that this trick with s_pInfoEx
> which makes it much more complicated then necessary is workaround
> for GPF at application startup caused by accessing TSD before HVM
> stack is initialized. It seems to work though it can be greatly
> simplified and of course the GPF in startup code and non HVM threads
> has to be fixed.
That pretty much sums up what I didn't like about the
implementation without even knowing the exact details
(I'd have bet that MT mode had problems f.e.)
And while I liked Przemek's HB_XGRABFM() alternative
to the problem, what I don't understand is why are we
trying to solve these problems (like the C++ variation)
at all on the Harbour core level?
First of all our goal is not to create a full-blown
memory analyzer. Secondly it's not trivial task and
has its risks since it resides in the very core of
Harbour.
IMO, however hard we try we will never even reach
existing memory tracing tools.
BTW Valgrind does its just pretty well, and it does
much more than detecting leaks. Unfortunately it
cannot run on all Harbour platforms, but since Harbour
is portable, most of its parts can be tested in
Valgrind given a free VirtualBox with a free Linux
in it. The whole dev environment can be installed
in 1-2 hours.
Or, we could also look around for Windows-compatible
solution from this list:
http://en.wikipedia.org/wiki/Memory_debugger
or elsewhere.
Brgds,
Viktor
_______________________________________________
Harbour mailing list (attachment size limit: 40KB)
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour