This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Mark, thanks for the patch. Generally the patch is fine, I have just a few nits. On Feb 16 21:28, Mark Geisert wrote: > I've attached a patch set modifying Cygwin's profiling support to sample PC > values of all an application's threads, not just the main thread. There is > no change to how profiling is requested: just compile and link the app with > "-pg" as usual. The profiling info is dumped into file gmon.out as usual. > > There is a behavioral change that ought to be documented somewhere: If a If it ought to be documented, what about providing the doc patch, too? Any chance you could come up with a short section about profiling in the context of winsup/doc/programming.xml? Otherwise there's basically only the description of the ssp tool in winsup/doc/utils.xml yet, which is a bit ... disappointing. > diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din > index 9584d09..243fd01 100644 > --- a/winsup/cygwin/common.din > +++ b/winsup/cygwin/common.din > @@ -269,6 +269,7 @@ ctime SIGFE > ctime_r SIGFE > cuserid NOSIGFE > cwait SIGFE > +cygheap_profthr_all NOSIGFE I would like to avoid exporting new cygwin-internal symbols. Could you please wrap the new functionality in a call to cygwin_internal? Just add a new CW_xxx symbol to include/sys/cygwin.h and use that from profthr_func. > +extern "C" void > +cygheap_profthr_all (void (*profthr_byhandle) (HANDLE)) > +{ > + int ix = -1; > + while (++ix < (int) nthreads) Why the cast? Why not stick to the type of nthreads, e.g. for (uint32_t ix = 0; ix < nthreads; ++ix) > + { > + _cygtls *tls = cygheap->threadlist[ix].thread; > + if (tls->tid) > + profthr_byhandle (tls->tid->win32_obj_id); > + } > +} > @@ -49,6 +50,7 @@ static char rcsid[] = "$OpenBSD: gmon.c,v 1.8 1997/07/23 21:11:27 kstailey Exp $ > > /* XXX needed? */ > //extern char *minbrk __asm ("minbrk"); > +extern int _setmode(int, int); > > #ifdef _WIN64 > #define MINUS_ONE_P (-1LL) > @@ -152,6 +154,7 @@ void > _mcleanup(void) > { > static char gmon_out[] = "gmon.out"; > + static char gmon_template[] = "gmon.outXXXXXX"; > int fd; > int hz; > int fromindex; > @@ -222,7 +225,14 @@ _mcleanup(void) > proffile = gmon_out; > #endif > > - fd = open(proffile , O_CREAT|O_TRUNC|O_WRONLY|O_BINARY, 0666); > + fd = open(proffile, O_CREAT|O_EXCL|O_TRUNC|O_WRONLY|O_BINARY, 0666); > + if (fd < 0 && errno == EEXIST) { > + fd = mkstemp(gmon_template); > + if (fd >= 0) { > + _setmode(fd, O_BINARY); You don't have to call _setmode here. Files created with mkstemp are O_BINARY anyway. And if you don't trust it, use mkostemp with an explicit O_BINARY flag. > static void CALLBACK > profthr_func (LPVOID arg) > { > struct profinfo *p = (struct profinfo *) arg; > - size_t pc, idx; > > for (;;) > { > - pc = (size_t) get_thrpc (p->targthr); > - if (pc >= p->lowpc && pc < p->highpc) > - { > - idx = PROFIDX (pc, p->lowpc, p->scale); > - p->counter[idx]++; > - } > + // record profiling sample for main thread > + profthr_byhandle (p->targthr); > + > + // record profiling samples for other pthreads, if any > + cygheap_profthr_all (profthr_byhandle); As outlined above, please call `cygwin_internal (CW_foo, profthr_byhandle)' from here. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |