[PATCH] gprof profiling of multi-threaded Cygwin programs, ver 2
Jon Turney
jon.turney@dronecode.org.uk
Mon Feb 22 11:44:00 GMT 2016
Thanks for this. A few comments inline.
On 20/02/2016 08:16, Mark Geisert wrote:
> diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc
> index 6493485..4932cf0 100644
> --- a/winsup/cygwin/cygheap.cc
> +++ b/winsup/cygwin/cygheap.cc
> @@ -744,3 +744,15 @@ init_cygheap::find_tls (int sig, bool& issig_wait)
> WaitForSingleObject (t->mutex, INFINITE);
> return t;
> }
> +
> +/* Called from profil.c to sample all non-main thread PC values for profiling */
> +extern "C" void
> +cygheap_profthr_all (void (*profthr_byhandle) (HANDLE))
> +{
> + for (uint32_t ix = 0; ix < nthreads; ix++)
> + {
> + _cygtls *tls = cygheap->threadlist[ix].thread;
> + if (tls->tid)
> + profthr_byhandle (tls->tid->win32_obj_id);
> + }
> +}
There doesn't seem to be anything specific to profiling about this, so
it could be written in a more generic way, as "call a callback function
for each thread".
> diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
> index e379df1..02335eb 100644
> --- a/winsup/cygwin/external.cc
> +++ b/winsup/cygwin/external.cc
> @@ -702,6 +702,17 @@ cygwin_internal (cygwin_getinfo_types t, ...)
> }
> break;
>
> + case CW_CYGHEAP_PROFTHR_ALL:
> + {
> + typedef void (*func_t) (HANDLE);
> + extern void cygheap_profthr_all (func_t);
> +
> + func_t profthr_byhandle = va_arg(arg, func_t);
> + cygheap_profthr_all (profthr_byhandle);
> + res = 0;
> + }
> + break;
> +
Is this exposed via cygwin_internal() operation just for testing
purposes? Or is there some other use envisioned?
> + /* We copy an undocumented glibc feature: customizing the profiler's
> + output file name somewhat, depending on the env var GMON_OUT_PREFIX.
> + if GMON_OUT_PREFIX is unspecified, the file's name is "gmon.out".
> +
> + if GMON_OUT_PREFIX is specified with at least one character, the
> + file's name is computed as "$GMON_OUT_PREFIX.$pid".
> +
> + if GMON_OUT_PREFIX is specified but contains no characters, the
> + file's name is computed as "gmon.out.$pid". Cygwin-specific.
> + */
> + if ((prefix = getenv("GMON_OUT_PREFIX")) != NULL) {
setup-env.xml might be an appropriate place to mention this environment
variable.
> 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
> + cygwin_internal (CW_CYGHEAP_PROFTHR_ALL, profthr_byhandle);
> +
Hmm.. so why isn't this written as cygheap_profthr_all (profthr_byhandle) ?
More information about the Cygwin-patches
mailing list