krime / gperftools

Automatically exported from code.google.com/p/gperftools
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

gperftools should intercept pthread_create and call ProfilerRegisterThread #705

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This is a feature request.

I recently found -- while using CPUPROFILE_TIMER_SIGNAL :-) -- that profiling 
multi-threaded programs using per-thread timers does not work because only the 
main thread receives the timer signal.

I eventually found that per-thread timers require calling 
ProfilerRegisterThread() from inside of each thread.

But one of the great features of gperftools is that you can use it without 
compiling against it; indeed without recompiling at all. I want to preserve 
that capability.

So, I wrote a little shared library gadget that intercepts calls to 
pthread_create() and calls ProfilerRegisterThread(). (The only slightly tricky 
part is calling the register function inside the new thread.) I am attaching my 
gadget to this report. To use it, you just "gcc -O -fpic -shared -o 
pthread-gperftools-hook.so pthread-gperftools-hook.c", then add it to 
LD_PRELOAD when using gperftools (e.g. 
LD_PRELOAD=/path/to/pthread-gperftools-hook.so:/path/to/libprofiler.so).

This is working for me, but I think something like this should be part of 
gperftools by default. Obviously it would need to work for programs not linked 
against -lpthread, and it would need to be controlled by new configure options 
and/or environment variables.

If you agree this would be a nice feature to have, and you can briefly 
summarize how you think it should work, I would be glad to take a crack at 
implementing it.

Thanks!

Original issue reported on code.google.com by lopre...@gmail.com on 4 Aug 2015 at 9:33

Attachments:

GoogleCodeExporter commented 9 years ago
I wrote similar thing too: https://github.com/alk/gperf-all-threads

My only issue with this approach is that it won't work with fully statically 
linked programs. I.e. they cannot use libdl.

This is kinda unpopular and discouraged option, but clearly still important to 
support.

Basically we should compile and work for at least those options (as noted below 
I'm not certain we really need all of them for libprofiler):

* position-independent code linked into .so

* .a linked statically into program that is otherwise linking to libpthread & 
libc dynamically

* .a linked statically into program that links everything statically

I'm also not sure about overhead of creating timer for every thread.

So if you can prepare some specific gperftools patch that does something about 
static linking (perhaps by trying to access libdl functions via weak symbols 
and doing nothing if they are missing), I'll surely want to merge it. Or maybe 
it will be fine to just always assume that libprofiler has to be .so. In any 
case, as long as patch looks decent, feel free to submit it.

Original comment by alkondratenko on 5 Aug 2015 at 4:42

GoogleCodeExporter commented 9 years ago
Wow, even the function names are similar... Great minds think alike?

Yours is cleaner, though.

The static linking case is tricky. The new "pthread_create" function itself has 
to be weak to avoid having it multiply defined when statically linked. I just 
tried a test, and LD_PRELOAD appears to interpose weak symbols just fine, so we 
can run with that. It does mean the pthread_create() intercept itself will be 
avoided when statically linked, but I think that is unavoidable.

OK I will take a crack at a patch. Although probably not until the weekend.

Thanks for the quick response.

Original comment by lopre...@gmail.com on 5 Aug 2015 at 5:53