From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13207 invoked by alias); 28 Oct 2019 15:32:26 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 13194 invoked by uid 89); 28 Oct 2019 15:32:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.2 required=5.0 tests=AWL,BAYES_00,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.1 spammy=asap, ASAP, pool, uct X-Spam-Status: No, score=-11.2 required=5.0 tests=AWL,BAYES_00,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: mx0b-0010f301.pphosted.com Received: from mx0b-0010f301.pphosted.com (HELO mx0b-0010f301.pphosted.com) (148.163.153.244) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Oct 2019 15:32:23 +0000 Received: from pps.filterd (m0102859.ppops.net [127.0.0.1]) by mx0b-0010f301.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id x9SFKfwF011806; Mon, 28 Oct 2019 10:32:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rice.edu; h=date : from : subject : to : cc : message-id : in-reply-to : references : mime-version : content-type; s=ricemail; bh=qonyJcPf4iRAfr1XJHsuvxzfxuJ0xirp4tPjo6xPoP0=; b=BbRbNdfFEvWHgmaCSSVdDcn6R72HlyLJVkwDs0zZsH8MBK7kR+B5wx9fVuniP011kQuk JohZVRYCwDezUIWjQDAu8aXCCa+fUxHfZlbxQJF5qL6iCzG6lrXkb17sTXhapPFpORC+ sk0GW0aOlY3hi2eFyMlpQ/3QaIleDzQz/EBPm/903ElDIKuxoLHc1O2QUUTLy2q3y8kZ WsYmQwrU7NdVBd0NJZ0mCYktVD1/IiyN56iNRuEHy1sqzfeAcxxeH/jX2WeZK6ssz/Wl 88QX/bEzW72lL6vmqZL7YjBl85qlkJ7uRsgK3G+W8hfePn5v+eC8jyryDMLCgAZbUumM Kw== Received: from mh3.mail.rice.edu (mh3.mail.rice.edu [128.42.199.10]) by mx0b-0010f301.pphosted.com with ESMTP id 2vvhgvsw0a-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Oct 2019 10:32:16 -0500 Received-X: from mh3.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh3.mail.rice.edu (Postfix) with ESMTP id 43D3E403F0; Mon, 28 Oct 2019 10:32:16 -0500 (CDT) Received-X: from mh3.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh3.mail.rice.edu (Postfix) with ESMTP id 4295D403E8; Mon, 28 Oct 2019 10:32:16 -0500 (CDT) X-Virus-Scanned: by amavis-2.7.0 at mh3.mail.rice.edu, auth channel Received-X: from mh3.mail.rice.edu ([127.0.0.1]) by mh3.mail.rice.edu (mh3.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id juFNEWjeU-uC; Mon, 28 Oct 2019 10:32:16 -0500 (CDT) Received: from cslinux29.cs.rice.edu (cslinux29.cs.rice.edu [168.7.116.104]) (using TLSv1.2 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: jma14) by mh3.mail.rice.edu (Postfix) with ESMTPSA id 0BAD4403E7; Mon, 28 Oct 2019 10:32:16 -0500 (CDT) Date: Mon, 28 Oct 2019 15:32:00 -0000 From: Jonathon Anderson Subject: Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev() To: Mark Wielaard Cc: Florian Weimer , elfutils-devel@sourceware.org, Srdan Milakovic Message-Id: <1572276735.2150.0@rice.edu> In-Reply-To: References: <1565983469.1826.0@smtp.mail.rice.edu> <20190824232438.GA2622@wildebeest.org> <1566695452.979.1@smtp.mail.rice.edu> <5ba06557703ee363e19488c994cbddd92ade25be.camel@klomp.org> <1566782688.5803.0@smtp.mail.rice.edu> <1566826627.5246.0@smtp.mail.rice.edu> <1566877968.10901.0@smtp.mail.rice.edu> <87tv7vg4l0.fsf@mid.deneb.enyo.de> <8fde453a2c570fa150aa39b0dabd0f925c7b0970.camel@klomp.org> X-Mailer: geary/3.34.1 MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8 definitions=2019-10-28_06:2019-10-28,2019-10-28 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 phishscore=0 mlxlogscore=500 lowpriorityscore=0 suspectscore=2 priorityscore=1501 malwarescore=0 clxscore=1015 mlxscore=0 spamscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000 definitions=main-1910280156 Content-Type: text/plain; charset=us-ascii; format=flowed X-SW-Source: 2019-q4/txt/msg00063.txt.bz2 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? >> >> From what I can tell, no. A quick google search indicates as such, >> and its even #defined as 1024 on my machine. > > I see, it is a hardcoded constant per architecture, but it seems every > architecture simply uses 1024. I am afraid that kind of rules out > having a pthread_key per Dwarf object. It is not that large a number. > > Programs are sometimes linked against 50 till 100 shared libraries, if > they use dwz/alt-files that means the potential open Dwarf objects is > several hundred already. It wouldn't be that crazy to have all of them > open at the same time. That might not reach the limit yet, but I think > in practice you could come close to half very easily. And with split- > dwarf every CU basically turns into a Dwarf object, which can easily > go > past 1024. > >> > There may be other ways to handle this, I'm high-level >> considering >> > > potential alternatives (with more atomics, of course). The >> > > difficulty >> > > is mostly in providing the same performance in the >> single-threaded >> > > case. >> > > >> > > > You already have a Dwarf *. I would consider adding some >> sort of >> > > > clone function which creates a shallow Dwarf * with its own >> > > embedded >> > > > allocator or something like that. >> > > >> > > The downside with this is that its an API addition, which we >> (the >> > > Dyninst + HPCToolkit projects) would need to enforce. Which >> isn't a >> > > huge deal for us, but I will need to make a case to those >> teams to >> > > make >> > > the shift. >> > > >> > > On the upside, it does provide a very understandable semantic >> in the >> > > case of parallelism. For an API without synchronization >> clauses, >> > > this >> > > would put our work back into the realm of "reasonably correct" >> (from >> > > "technically incorrect but works.") >> > >> > Could someone give an example of this pattern? >> > I don't fully understand what is being proposed and how the >> interface >> > would look exactly. >> >> An application would do something along these lines: >> >> Dwarf* dbg = dwarf_begin(...); >> Dwarf* dbg2 = dwarf_clone(dbg, ...); >> pthread_create(worker, ...); >> // ... >> dwarf_get_units(dbg, ...); >> // ... >> pthread_join(worker); >> dwarf_end(dbg); >> >> // worker: >> // ... >> dwarf_getabbrev(...); >> // ... >> dwarf_end(dbg2); >> >> The idea being that dbg2 and dbg share most of the same internal >> state, >> but concurrent access to said state is between Dwarfs (or >> "Dwarf_Views", maybe?), and the state is cleaned up on the >> original's >> dwarf_end. I suppose in database terms the Dwarfs are acting like >> separate "cursors" for the internal DWARF data. For this particular >> instance, the "top of stack" pointers would be in dbg and dbg2 (the >> non-shared state), while the atomic mem_tail would be part of the >> internal (shared) state. >> >> I'm not sure how viable implementing this sort of thing would be, it >> might end up overhauling a lot of internals, and I'm not familiar >> enough with all the components of the API to know whether there >> would >> be some quirks with this style. > > So they would have separate lazy DWARF DIE/abbrev readers and separate > allocators? And any abbrevs read in the clone would just be thrown > away > after a dwarf_end? Separate allocators but the same lazy DIE/abbrev readers, most likely. So a Dwarf would be split into the concurrent Dwarf_Shared and non-concurrent Dwarf "View", maybe something like: struct Dwarf_Shared { pthread_rwlock_t rwl; /* For all currently non-concurrent internals */ Elf *elf; char *debugdir; Dwarf *alt_dwarf; Elf_Data *sectiondata[IDX_last]; bool other_byte_order; bool free_elf; int alt_fd; struct pubnames_s { Dwarf_Off cu_offset; Dwarf_Off set_start; unsigned int cu_header_size; int address_len; } *pubnames_sets; size_t pubnames_nsets; void *cu_tree; /* Dwarf_Off next_cu_offset; // Moved to Dwarf View */ void *tu_tree; /* Dwarf_Off next_tu_offset; // Moved to Dwarf View */ Dwarf_Sig8_Hash sig8_hash; void *split_tree; void *macro_ops; void *files_lines; Dwarf_Aranges *aranges; struct Dwarf_CFI_s *cfi; struct Dwarf_CU *fake_loc_cu; struct Dwarf_CU *fake_loclists_cu; struct Dwarf_CU *fake_addr_cu; /* pthread_key_t mem_key; // Implemented differently */ atomic_uintptr_t shared_mem_tail; size_t mem_default_size; /* Dwarf_OOM oom_handler; // Moved to Dwarf View, maybe */ }; struct Dwarf /*View*/ { bool free_shared; /* If true, we handle cleaning up on dwarf_end */ struct Dwarf_Shared *shared; /* All cloned() Views share the same Shared in the back */ Dwarf_Off next_cu_offset; Dwarf_Off next_tu_offset; struct libdw_memblock *mem_tail; Dwarf_OOM oom_handler; }; So most everything is in Dwarf_Shared, and the bits that really only make sense when done in serial are part of the View. And then allocations are done from a View-local stack, while everything is pushed as is now onto the Shared stack for deallocation. > > In general I think this style is not as nice as having a shared state > of the Dwarf object that is only manipulated in a thread-safe manner. I tend to prefer styles like this only because they have a clean split to define what can and cannot be concurrent, even logically (What makes sense when iterating CUs from multiple threads? Does the OOM handler have to be thread-safe?). Otherwise you end up trying to answer an N^2 problem for how all the moving pieces interact (and it often requires knowledge of the code to know which pieces do interact). > You did have an earlier implementation that didn't use pthread_keys. > Maybe we should just fall back to that one? I can revive and rebase it, its on my list for doing ASAP after my other more pressing duties. It still has quirks that bother me a little (like not doing well without a thread pool), but it'll work fine. > > Thanks, > > Mark