Re: [PATCH] profile support

[jojelino <jojelino at gmail dot com>]

On 2011-08-20 PM 5:49, Corinna Vinschen wrote:
> Hi Jin-woo,
>
> On Aug 20 06:59, jojelino wrote:
> > On 2011-08-20 AM 6:48, jojelino wrote:
> > > cd i686-pc-cygwin/winsup/cygwin;rm *.o;make profile=1
> > >
> > > it would work.
> > Please ignore attachment from 6:48, i have mistaken by attaching
> > invalid winsup-profile-support.diff
> >
> > So i am replying.
>
> Yes, but to the wrong list ;-)
>
> Would you mind to resend your patches as a reply to your original
> threada on the cygwin-patches list?  That's where the official
> patch submissions are suppsoed to go.
>
> Btw., you don't have to attach the ChangeLog.  For the lazy reader
> it's easier if the ChangeLog is just inline in your mail.  If you
> want to avoid more spam, just omit your email address from the
> ChangeLog date/name/email header.
Yes, I see.
> But there's something I don't grok in your patch:
Yes, I need to spend more time.
> In dll_crt0_0:
>
>    +  _monstartup();
>
> but
>
>    +#ifdef PROFILE
>    +  atexit (&_mcleanup);
>    +#endif
>
> Shouldn't the call to _monstartup also be guarded with #ifdef PROFILE?
> Same question for calls to profile_thread_off and any other profiling
> code in other places.
Yes i was supposed to do that. thanks for pointing it out.
> In gcrt0.c:
>
>    +#ifndef PROFILE
>    void _monstartup (void) __attribute__((__constructor__));
>    -
>    +#endif
>
> #if*n*def?  Is that a typo?
No, it's not typo. it is assumed that PROFILE is defined iff make is 
invoked with profile=1, and when PROFILE defined, the patch need to have 
different invocation of _monstartup. the only reason doing this is that 
the patch make use of sec_none_nih on profile_on.
and diff you mentioned in gcrt0.c doesn't exist any longer.

changelog of winsup-profile-support-2.diff
Following patch are not effective unless you 'make profile=1'
	
	* profil.h: (struct profinfo::{queue,operational}): New member.
	(struct clk): New struct.
	* gmon.h: (struct gmonparam::comm_kcount): New member.
	* gmon.c: (s_scale): Add NO_COPY_INIT.
	(_gmonparam): Ditto.
	* profil.c: (prof): Ditto.
	(profile_off): Finish profiling request.
	(apc_spawnthread): New function.
	(profile_on): Initialize init_global_security prior to use it, and 
spawn the consumer thread by issuing APC call.
	(fake_sbrk): Use LocalAlloc instead of malloc.
	(monstartup): Claim memory for comm_kcount.
	(_mcleanup): Alter formatting of cleanup filename, decide hz which 
maximizes kcount, calculate kcount using comm_kcount. 	
	* gcrt0.c (PROFILE): Define.
	(_monstartup): Use it.
	* exceptions.cc (ctrl_c_handler): Stop the consumer thread explicitly 
to avoid deadlock.
	* external.cc (exit_process): Ditto.
	* pinfo.cc (pinfo::exit): Ditto.
	* init.cc (respawn_wow64_process): Ditto.
	 (dll_entry): call __cyg_profile_{func,tls}_{ctor,dtor} to initialize 
instrument function.
	* dcrt0.cc (dll_crt0_0): call _monstartup,atexit in proper order,
	* Makefile.in (EXTRA_DEPENDENCIES_1): Define variable.
	(EXTRA_LDFLAGS_1): Ditto.
	(profile): Ditto
	(CFLAGS): Redefine.
	(gcrt1.o): use gcrt0.c as prerequisite.
	(TEST_DLL_NAME): use EXTRA_DEPENDENCIES_?, EXTRA_LDFLAGS_?.
	* sec_helper.cc (init_global_security): Check if called twice.
	* boundbuffer.c: New file.
	* boundbuffer.h: Ditto.
	* instrument.c: Ditto.
	* instrument.h: Ditto.
changelog of winsup-makefile.diff
	* Makefile.common (ALL_CFLAGS): Replace simple expansion to recursive 
expansion.

Attachment: winsup-profile-support-2.diff
Description: Text document

Attachment: winsup-makefile.diff
Description: Text document