gprof profiling of multi-threaded Cygwin programs

[Mark Geisert]

I've got alpha-quality code running to support this and am running some
tests to ensure I'm seeing what I ought to be seeing. Could I get a quick
design review to confirm I'm on the right track of implementation and
to maybe point out odd use cases I might have missed?

I could have just submitted patches but I'd like to be more sure of my
direction before patching.

I see three areas of implementation...
(1) Modify profil.c to have the profiling thread it creates sample all
pthreads' pc values in addition to its current sampling of the main
thread's. I plan to have profil.c call a routine in cygheap.cc to get
handles to the running pthreads that can be passed to GetThreadContext()
back in profil.c. (The code is cleaner than this reads.)

(2) Modify mcount.c to make mcount_private() thread-safe. This routine is
entered on every function call that occurs in an executable compiled with
the '-pg' option; this to build call path diagrams. I plan to replace the
sets and refs of the gmonparam state variable with Interlocked*
operations. It would be nice to record somewhere how many misses occur;
the code doesn't queue callers because it's being called on *every*
profiled function call. That happens so very frequently under profiling
that if the state doesn't allow entry that call's info is just discarded.

(3) Modify gmon.c to allow for possibility of multiple gmon.out files due
to fork/exec from a profiled process. I see there is potentially usable
code already there but it's #ifdef'd out. Does anybody know why?

Thanks for the once-over and if submitting patches really is the way I
should go for this review, just let me know.

..mark

[Corinna Vinschen]

Hi Mark,

Post by Mark Geisert
I've got alpha-quality code running to support this and am running some
tests to ensure I'm seeing what I ought to be seeing. Could I get a quick
design review to confirm I'm on the right track of implementation and to
maybe point out odd use cases I might have missed?
I could have just submitted patches but I'd like to be more sure of my
direction before patching.

Unless your patches are trivial (no new functionality, less than 10
lines), submitting patches requires a copyright assignement, See
https://cygwin.com/contrib.html especially the "Before you get started"
section.

Post by Mark Geisert
I see three areas of implementation...
(1) Modify profil.c to have the profiling thread it creates sample all
pthreads' pc values in addition to its current sampling of the main
thread's. I plan to have profil.c call a routine in cygheap.cc to get
handles to the running pthreads that can be passed to GetThreadContext()
back in profil.c. (The code is cleaner than this reads.)

Sounds ok to me. Try to keep it so that code isn't slowed down when
profiling is *not* used.

Post by Mark Geisert
(2) Modify mcount.c to make mcount_private() thread-safe.

Yuk. Thread-safety wasn't on the plate at all when this code has been
written I guess...

Post by Mark Geisert
entered on every function call that occurs in an executable compiled with
the '-pg' option; this to build call path diagrams. I plan to replace the
sets and refs of the gmonparam state variable with Interlocked* operations.
It would be nice to record somewhere how many misses occur; the code doesn't
queue callers because it's being called on *every* profiled function call.
That happens so very frequently under profiling that if the state doesn't
allow entry that call's info is just discarded.

Yeah, sounds fine to me.

Post by Mark Geisert
(3) Modify gmon.c to allow for possibility of multiple gmon.out files due to
fork/exec from a profiled process. I see there is potentially usable code
already there but it's #ifdef'd out. Does anybody know why?

Sorry, no. The gist of that code is basically written the last millenium.
Some of the ifdef's/ifndef's probably have been added just to simplify the
code (or something didn't work in 1997 or so). Feel free to change that,
but try to keep it as much aligned to some version of upstream as possible,
with upstream being one of FreeBSD, NetBSD, OpenBSD.

Post by Mark Geisert
Thanks for the once-over and if submitting patches really is the way I
should go for this review, just let me know.

Well, it's kind of the next step, isn't it? ;)


Thanks,
Corinna

[Mark Geisert]

Post by Corinna Vinschen

Post by Mark Geisert
I could have just submitted patches but I'd like to be more sure of my
direction before patching.

Unless your patches are trivial (no new functionality, less than 10
lines), submitting patches requires a copyright assignement, See
https://cygwin.com/contrib.html especially the "Before you get started"
section.

D'oh! I must have seen those instructions go by dozens of times yet
failed to realize they applied to me now. I've sent in the assignment.

..mark

[Corinna Vinschen]

Hi Mark,

Post by Corinna Vinschen

Post by Mark Geisert
I could have just submitted patches but I'd like to be more sure of my
direction before patching.

Unless your patches are trivial (no new functionality, less than 10
lines), submitting patches requires a copyright assignement, See
https://cygwin.com/contrib.html especially the "Before you get started"
section.

D'oh! I must have seen those instructions go by dozens of times yet failed
to realize they applied to me now. I've sent in the assignment.

Received and countersigned.


Thanks,
Corinna