Re: [Mldonkey-devel] RFC : logging in mldonkey

[Fortin Denis]

Le dim, 09/05/2004 à 22:08 +0200, spiral voice a écrit :
> Fortin Denis wrote:
> >I would like to discuss the way we are logging messages in mldonkey.
> 
> Hi,
> 
> I began testing your patch but I found some stuff missing.
> As I compile i386 cores on a i686 machine I have to do this before 
> ./configure
> 
> cd config; cp Makefile.config.i386.in Makefile.config.in; cd ..
> 
> You added
> address@hidden@
> to config/Makefile.config.in but not to the CPU depending files, the 
> attached patch solves this.
> Without it the core does not compile!
Thanks. I must admit that i didn't try to compile mldonkey with all those 
options.

> When compiling DirectConnect and Opennapster modules this happens,
> CVS 2-5-21 with patch 3039, the attached patch and logging.patch was used:
[...]
> make: *** [src/networks/opennap/opennapProtocol.cmx] Error 2

camlp4 parser seems to be more strict than ocaml parser.
In both case you should remove the bogus Msg module without problems.


> Calling ocamlopt -verbose shows this:
> + camlp4o ./tools/pa_log.cmo q_MLast.cmo -VDEBUG 
> src/networks/opennap/opennapProtocol.ml > /tmp/camlpp875462
> File "src/networks/opennap/opennapProtocol.ml", line 774, characters 14-15:
> 
> After disabling both networks, I am getting to the newgui part. Without
> export OCAMLRUNPARAM="l=256M"
> the splash screen of Newgui does not compile:
> 
> ocamlopt -inline 10    -I src/utils/cdk  -I src/daemon/chat  -I 
> src/utils/lib  -I src/utils/net  -I tools  -I src/daemon/common  -I 
> src/daemon/driver  -I src/utils/mp3tagui  -I src/config/unix  -I 
> src/gtk/newgui  -I src/gtk/gui2  -I src/gtk/configwin  -I src/gtk/okey  -I 
> src/gtk/gpattern  -I icons/tux  -I +lablgtk  -I src/gtk/progress  -I src/im  
> -I src/im/yahoo  -I src/im/irc  -I src/networks/gnutella  -I 
> src/networks/gnutella2  -I src/networks/fasttrack  -I src/networks/fileTP  
> -I src/networks/bittorrent  -I src/networks/donkey  -I src/networks/soulseek 
> -I +camlp4 -pp "camlp4o ./tools/pa_log.cmo q_MLast.cmo -VDEBUG" -c 
> icons/tux/splash_screen_xpm.ml
> File "icons/tux/splash_screen_xpm.ml", line 3904, characters 14-15:
> Uncaught exception: Stack_overflow
> Preprocessor error

This is Makefile nightmare. I shouldn't preprocess these files. 
I don't know if i can make a separate target for _xpm.ml files (or even
if it's possible).
I will try to fix that if we decide to go with this patch.


> This did not happen before and seems to be related to the debugging macros 
> included in the compile command.
> 
> Another question, what are your plans about the existing logging verbosity:
> 
>         (* A space-separated list of keywords. Each keyword triggers
>           printing information on the corresponding messages:
>           mc : debug client messages
>           mct : debug emule clients tags
>           ms : debug server messages
>           connect : debug connections
>           net : debug net
>           verb : debug other
>           loc : debug source research
>           sp : debug source propagation
>           sm : debug source management
>           do : some download warnings
>           up : some upload warnings
>           unk : unknown messages
>           ov : overnet
>           share: debug sharing
>           md4 : md4 computation
>           udp : udp messages
>           hc: http_client messages
>           hs: http_server messages
>          *)
> verbosity = ""
> 
> I like the way I can switch on context depended logging, for example mct to 
> hunt for new emule mods. How does this fit into your new logging scheme? 
> Should this verbosity output function and all its dependencies only be 
> enabled if --enable-verbosity=debug? This would be my suggestion.

The plan is to keep them. 
You should use BLOG DEBUG (!!verbosity) : "fmt" args ELOG; 

> But what about the debug_client stuff? This should IMHO be available not 
> only in --enable-verbosity=debug, but also in the standard 
> --enable-verbosity=info.
Since it's a debug function i would put it in the debug level like this
BLOG  DEBUG (c.client_debug) #
     BLOG DEBUG : "Client[%d]: disconnected" (client_num c) ELOG;
     CommonGlobals.print_localtime ();
ELOG;
but you can also decide to put it in info level.

> Or are you planning to leave some debug stuff the way it currently is?

I didn't remove lprintf so in anycase it can remain as it is.

Do you think that the potential performance improvement worth the
trouble of macros?

Denis