From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71105 invoked by alias); 24 Feb 2020 02:52:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 71093 invoked by uid 89); 24 Feb 2020 02:52:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=forces, H*f:sk:87y2ss5, H*i:sk:87y2ss5, H*MI:sk:87y2ss5 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Feb 2020 02:52:07 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 9D3C71E5FA; Sun, 23 Feb 2020 21:52:03 -0500 (EST) Subject: Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20200215165444.32653-1-tom@tromey.com> <87y2ss5019.fsf@tromey.com> From: Simon Marchi Message-ID: <8b11c5f6-771f-69b5-c98c-94ced592132f@simark.ca> Date: Mon, 24 Feb 2020 02:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <87y2ss5019.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-02/txt/msg00909.txt.bz2 On 2020-02-23 6:58 p.m., Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> I think it would be more natural and easier to understand to have : > > Simon> objfile -> dwarf2 objfile-specific object -> dwarf2 object shared between objfiles > > Simon> Many "dwarf2 objfile-specific" objects would point to one "dwarf2 object shared between > Simon> objfiles", without special tricks. Of course, it's much easier said than done. > > Simon> I'd like to know: did you consider this way of doing it and chose not to do it > Simon> because of the complexity / size of the change? Is the current patchset a stepping > Simon> stone towards it? > > I was afraid of the size of the resulting patch. However, I think > you're right that this would be better. And, I feel pretty strongly > that we should be making the DWARF reader less awful, not more. Yeah, as I already showed you, I started making such a patch (more details below). I think it's worth doing this work, because in the end it's easier to follow the code. I find that the dwarf2_enter_objfile approach is analogous to having to set a "current context" (e.g. call switch_to_thread) before calling a certain function, which goes against what we are trying to do in general (for example, trying to change functions to accept a thread / ptid rather than rely on inferior_ptid). Since dwarf2_per_cu_data objects become objfile-independent, functions that need to work on a dwarf2_per_cu_data in an objfile-dependent way then need to accept an objfile or dwarf2_per_objfile object along with the dwarf2_per_cu_data. It's a bit tedious, but in the end it forces us to pass the objfile context all the way from the callers, and it's less likely that we forget a spot, like we could with dwarf2_enter_objfile, since it would just not compile. > Probably the thing to do is reorder the series a little (some patches > don't depend on this), then redo the "unsharing" patch, and finally > update the subsequent patches. > > A few of these patches (1-3, 5, and 11) could probably go in sooner. > What do you think of that? #1 is already in, right? #2: Yes, sounds like a good cleanup. #4: Yes, good cleanup. #3: It's only useful if we end up doing psymtabs sharing, so I would not push it yet. #5 and #11: I'm not sure how much of that I end up un-doing in my prototype patch, swapping the shareable and unshareable parts. And it is also only useful in the context of psymtab sharing, so I wouldn't push it yet. I'm currently experimenting with what I suggested above, this is what I have for now (this will get force-pushed as I progress): https://github.com/simark/binutils-gdb/commits/share-psymtabs The status is: I'm based on your patch 9/14 ("Add objfile member to DWARF batons"), as it aims mainly to replace patch patch 10/14 ("Introduce dwarf2_enter_objfile and use it") with the architecture I proposed. I see no regression in a simple "make check" between it and the previous commit. I have yet to apply and adapt the rest of your series on top of my commit. I would also like to test with the variety of testsuite boards that Tom de Vries added, since they will test various DWARF configurations (and I would feel bad if he had to clean up after us again). Simon