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) ?



Reply via email to