Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Mon, Oct 28, 2019 at 14:26, Mark Wielaard wrote: On Sat, 2019-10-26 at 19:56 -0500, Jonathon Anderson wrote: On Sun, Oct 27, 2019 at 00:50, Mark Wielaard > wrote: > > I see that getconf PTHREAD_KEYS_MAX gives 1024 on my machine. > Is this tunable in any way?

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Sat, 2019-10-26 at 19:56 -0500, Jonathon Anderson wrote: > On Sun, Oct 27, 2019 at 00:50, Mark Wielaard wrote: > > > > I see that getconf PTHREAD_KEYS_MAX gives 1024 on my machine. > > Is this tunable in any way? > > From what I can tell, no. A quick google search indicates as such, > and it

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

* Jonathon Anderson: > On Sun, Oct 27, 2019 at 09:59, Florian Weimer wrote: >> * Mark Wielaard: >> Current glibc versions have a thread-local fast path, which should address some of these concerns. It's still not a bump-pointer allocator, but at least there are no atomics on t

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Sun, Oct 27, 2019 at 09:59, Florian Weimer wrote: * Mark Wielaard: Current glibc versions have a thread-local fast path, which should address some of these concerns. It's still not a bump-pointer allocator, but at least there are no atomics on that path. Since which version of gli

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

* Mark Wielaard: >> Current glibc versions have a thread-local fast path, which should >> address some of these concerns. It's still not a bump-pointer >> allocator, but at least there are no atomics on that path. > > Since which version of glibc is there a thread-local fast path? It was added i

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Sun, Oct 27, 2019 at 00:50, Mark Wielaard wrote: Hi, On Sat, 2019-10-26 at 11:45 -0500, Jonathon Anderson wrote: For some overall perspective, this patch replaces the original libdw allocator with a thread-safe variant. The original acts both as a suballocator (to keep from paying the

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi, On Sat, 2019-10-26 at 18:50 +0200, Florian Weimer wrote: > * Jonathon Anderson: > > > > This assumes that memory allocation > > > is actually a performance problem, otherwise you could let malloc > > > handle the details. > > > > In our (Dyninst + HPCToolkit) work, we have found that malloc

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi, On Sat, 2019-10-26 at 11:45 -0500, Jonathon Anderson wrote: > For some overall perspective, this patch replaces the original libdw > allocator with a thread-safe variant. The original acts both as a > suballocator (to keep from paying the malloc tax on frequent small > allocations) and a ga

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

* Jonathon Anderson: >> This assumes that memory allocation >> is actually a performance problem, otherwise you could let malloc >> handle the details. > > In our (Dyninst + HPCToolkit) work, we have found that malloc tends to > be slow in the multithreaded case, in particular with many small >

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hello Florian Weimer, I'm the original author of this patch, so I'll try to answer what I can. For some overall perspective, this patch replaces the original libdw allocator with a thread-safe variant. The original acts both as a suballocator (to keep from paying the malloc tax on frequent sma

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

* Mark Wielaard: > I'll see if I can create a case where that is a problem. Then we can > see how to adjust things to use less pthread_keys. Is there a different > pattern we can use? It's unclear what purpose thread-local storage serves in this context. You already have a Dwarf *. I would consi

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Sat, 2019-10-26 at 12:54 +0200, Florian Weimer wrote: > * Jonathon Anderson: > > > Just finished some modifications to the patch series, git request-pull > > output below. This rebases onto the latest master and does a little > > diff cleaning, the major change is that I swapped out the memor

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

* Jonathon Anderson: > Just finished some modifications to the patch series, git request-pull > output below. This rebases onto the latest master and does a little > diff cleaning, the major change is that I swapped out the memory > management to use the pthread_key_* alternative mentioned befo

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi Jonathan, On Mon, 2019-08-26 at 22:52 -0500, Jonathon Anderson wrote: > Just finished some modifications to the patch series, git request- > pull output below. This rebases onto the latest master and does a > little diff cleaning, the major change is that I swapped out the > memory management t

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hello Mark, Just finished some modifications to the patch series, git request-pull output below. This rebases onto the latest master and does a little diff cleaning, the major change is that I swapped out the memory management to use the pthread_key_* alternative mentioned before. I did some

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Looks correct to me (assuming it applies). I think there's another latent bug in there somewhere (tests that use libdwfl used to leak mem_tails, but now that dwarf_begin_elf doesn't do an initial malloc it doesn't trigger), I'll try hunting it down when I have the time. Glad I could be of help

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi Jonathon, Thanks for checking our new mailinglist settings :) You message was accepted now and the HTML attachment stripped. On Sun, 2019-08-25 at 20:24 -0500, Jonathon Anderson wrote: > The one extra line in dwarf_getcfi.c > is to account for a failure in the test suite when Valgrind is > ena

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hello Mark, Next iteration of the patch(es) are ready, hosted on Github (git request-pull output below). I think most of the issues we've discussed have been addressed; the setup with stdatomic.h is a little WIP, I wasn't certain how it should be done moving forward (and, I'm not all that fam

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi Jonathon, On Sat, 2019-08-24 at 20:10 -0500, Jonathon Anderson wrote: > On Sat, Aug 24, 2019 at 6:24 PM, Mark Wielaard wrote: > > But what if realloc moves the block? > > Then all dbg->mem_tails[thread_id] pointers become invalid. > > And this function drops the lock before returning a libdw_m

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Sat, Aug 24, 2019 at 6:24 PM, Mark Wielaard wrote: Hi, On Fri, Aug 16, 2019 at 02:24:28PM -0500, Jonathon Anderson wrote: 2. Adding a lock & array structure to the memory manager (pseudo-TLS) (libdwP.h, libdw_alloc.c) Specifically concentrating on this part. diff --git a/libdw/

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi, On Fri, Aug 16, 2019 at 02:24:28PM -0500, Jonathon Anderson wrote: > 2. Adding a lock & array structure to the memory manager (pseudo-TLS) > (libdwP.h, libdw_alloc.c) Specifically concentrating on this part. > diff --git a/libdw/ChangeLog b/libdw/ChangeLog > index bf1f4857..87abf7a7 100644

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Fri, Aug 23, 2019 at 1:25 PM, Mark Wielaard wrote: Hi, On Wed, 2019-08-21 at 09:08 -0500, Jonathon Anderson wrote: On Wed, Aug 21, 2019 at 6:16 AM, Mark Wielaard wrote:On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: > > For parallel applications that need the information

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Wed, 2019-08-21 at 17:21 -0500, Jonathon Anderson wrote: > > P.S. It looks like something decided to add some line breaks in the > > patch so that it doesn't easily apply. It isn't hard to fixup, but you > > might want to consider submitting using git send-email or attaching > > the result of g

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Wed, 2019-08-21 at 09:20 -0500, Jonathon Anderson wrote: > First message failed to send, hopefully this one works... Just for the record, the mailinglist did reject HTML messages/attachments. It should have been changed now to simply strip the HTML. Cheers, Mark

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi, On Wed, 2019-08-21 at 09:08 -0500, Jonathon Anderson wrote: > On Wed, Aug 21, 2019 at 6:16 AM, Mark Wielaard > wrote:On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: > > > For parallel applications that need the information in the DIEs, the > > > Dwarf_Abbrev hash table et al. be

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Wed, Aug 21, 2019 at 4:50 PM, Mark Wielaard wrote: On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: diff --git a/ChangeLog b/ChangeLog index bed3999f..93907ddd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2019-08-15 Jonathon Anderson + + * configure.ac:

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Wed, 2019-08-21 at 23:50 +0200, Mark Wielaard wrote: > On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: > > @@ -668,6 +697,7 @@ AC_MSG_NOTICE([ > >OTHER FEATURES > > Deterministic archives by default : > > ${default_ar_deterministic} > > Native language support

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: > diff --git a/ChangeLog b/ChangeLog > index bed3999f..93907ddd 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,8 @@ > +2019-08-15 Jonathon Anderson > + > + * configure.ac: Add new --enable-valgrind-annotations > + * conf

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

First message failed to send, hopefully this one works... On Wed, Aug 21, 2019 at 6:16 AM, Mark Wielaard wrote: Hi Jonathon and Srđan, On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: For parallel applications that need the information in the DIEs, the Dwarf_Abbrev hash table et

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

Hi Jonathon and Srđan, On Fri, 2019-08-16 at 14:24 -0500, Jonathon Anderson wrote: > For parallel applications that need the information in the DIEs, the > Dwarf_Abbrev hash table et al. become a massive data race. This fixes > that by: > > 1. Adding atomics & locks to the hash table to manage c