From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id zPhaDMmk7mhVVTIAWB0awg (envelope-from ) for ; Tue, 14 Oct 2025 15:30:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1760470217; bh=vX3t8HPjgKF3fgHecorAanyv/Wz9lr7hRhbBWvzynPQ=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=Qf0mZZ5f309FTqnN5JlFlVVL/YGhrrf7jch9BQScGw0FaSbe1bDK5drBTzSqVyelu zcIMYFlNnz4QNBuMvsmkecJGbtLkvPQVyBe9/TczxbgbDlaCFaIvREgTvHSxFAsAKn LhgtSOwZTCOlnNFKiNNyomRw5A222q/C2y+l5FG0= Received: by simark.ca (Postfix, from userid 112) id 235A91E0B6; Tue, 14 Oct 2025 15:30:17 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=MFvrMgW0; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 6EE591E047 for ; Tue, 14 Oct 2025 15:30:16 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1487F385841C for ; Tue, 14 Oct 2025 19:30:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1487F385841C Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=MFvrMgW0 Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B8BE53858D35 for ; Tue, 14 Oct 2025 19:29:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B8BE53858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B8BE53858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760470183; cv=none; b=xzK1ljtP2+gvK1cAourFkcFZb9/7SOOjM7YzhBT/bE+z4EssErhZqCfTpwzz7+ygeKhhlrKshoa7sKdNqA4Cw+TmWhoe3p+LYdOvuBbu06PjPQgd3R/F/HI2SKrIVa3ob9qyMa3MSFd+6kzDA7tEa0OAepyUjTtg1cYb834QQBo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760470183; c=relaxed/simple; bh=vX3t8HPjgKF3fgHecorAanyv/Wz9lr7hRhbBWvzynPQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=DN+0P65KyhmiIYlUpFUqkIkP+gdkkjL9cOqXj+VzRbcNp66b/zrmDUawjd/gGlufSKEAPFDiizqH7UmCJx4Ee4w8NcNzxP+WuqL5J97CzuWVqCZKG1gaJXL+jSFoixyL1PVA8xjLIiYriAJArjubDdg9ttcOCoTZIMfklJzkGoY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B8BE53858D35 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1760470183; bh=vX3t8HPjgKF3fgHecorAanyv/Wz9lr7hRhbBWvzynPQ=; h=Date:Subject:To:References:From:In-Reply-To:From; b=MFvrMgW0AAsvP+E6rtOMPEGDq5b2uYNDItbbnUAuby7hwlHEk1dbtTTOP4spjWXbY c5fV8lo/AHCEHB9CCs4/SACL91+Q+SD3wss2RnVXYt9p1Ffedp6H6YMrVRgdd/LFcK AB1A5ucatl7qxDXq94vGfvg3G8mgjWC8/zFdv9M4= Received: by simark.ca (Postfix) id 3E18F1E047; Tue, 14 Oct 2025 15:29:43 -0400 (EDT) Message-ID: <706dd0b5-a1f0-4cdb-bd41-b3b1a2bbac3a@simark.ca> Date: Tue, 14 Oct 2025 15:29:42 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] gdb: use std::vector<> to hold on blocks in struct blockvector To: Jan Vrany , gdb-patches@sourceware.org References: <20251013182318.1045138-1-jan.vrany@labware.com> <20251013182318.1045138-3-jan.vrany@labware.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20251013182318.1045138-3-jan.vrany@labware.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 10/13/25 2:23 PM, Jan Vrany wrote: > This commit changes internal implementation of struct blockvector to use > std::vector<> rather than flexible array, allowing one to easily add > blocks to existing blockvector. > > This is needed to for lazy CU expansion and for Python API to build > objfiles, compunits and symtabs dynamically (similarly to JIT reader > API). > > To avoid extensive reallocation when adding a lot of blocks, ctor takes > initial number of blocks to be stored in blockvector. > > While at is, it also adds blockvector::add_block() to add new blocks > and changes mdebugread.c to use it. > > The downside is higher memory consumption. The size of std::vector with > obstack allocator is 32 bytes (GCC 14) compared to 8 bytes used > currently to store the number of blocks (m_num_blocks). Stopping gdb at > its main(), followed by "maint expand-symtabs" results in 4593 > compunit symtabs so in this case the overhead is 24*4593 = 110232 bytes > which I hope is acceptable. > --- > gdb/block.c | 36 +++++++++++++++++++++++++++++ > gdb/block.h | 59 +++++++++++++++++++++++------------------------- > gdb/buildsym.c | 2 +- > gdb/jit.c | 2 +- > gdb/mdebugread.c | 30 +++--------------------- > 5 files changed, 69 insertions(+), 60 deletions(-) > > diff --git a/gdb/block.c b/gdb/block.c > index e09bccdcf86..a9191c3fdff 100644 > --- a/gdb/block.c > +++ b/gdb/block.c > @@ -17,6 +17,7 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see . */ > > +#include I'm not sure which change of yours requires cstring (perhaps this include has been auto-inserted, but if it's needed, I'd use , just for consistency with the rest of the codebase. > #include "block.h" > #include "symtab.h" > #include "symfile.h" > @@ -809,6 +810,41 @@ make_blockranges (struct objfile *objfile, > return blr; > } > > +static bool > +block_ordering_predicate (struct block *b1, struct block *b2) > +{ > + CORE_ADDR start1 = b1->start (); > + CORE_ADDR start2 = b2->start (); > + > + if (start1 != start2) > + return start1 < start2; > + return (b2->end () < b1->end ()); > +} > + > +/* See block.h. */ > + > +void > +blockvector::add_block (struct block *block) > +{ > + if (num_blocks () <= FIRST_LOCAL_BLOCK) > + { > + /* No blocks (except global and static block). */ > + m_blocks.push_back (block); > + } > + else > + { > + /* blockvector already contains some blocks. Insert new block > + to a correct place. */ Indentation is off. > + auto first = m_blocks.begin () + FIRST_LOCAL_BLOCK; > + auto last = m_blocks.end (); > + > + auto insert_before = std::upper_bound (first, last, block, > + block_ordering_predicate); > + > + m_blocks.insert (insert_before, block); > + } > +} This function makes me a bit worried, because I feel like it could (even though unlikely) end up mis-used and end up with a situation like this: https://inbox.sourceware.org/gdb-patches/b5853237-a041-4a74-aceb-0406eeb926ec@polymtl.ca/T/#m95998810ae7943611bec2a499070b735f6ec4928 That is, if you add some blocks not in the sorted order already and insert blocks in the middle of the vector, you end up shifting a lot of elements, and it can start to get noticeable. The only user of this function right now is mdebugread, and it didn't seem to need this "insert" in the middle approach. For that case, we could perhaps have an append_block method, which would have the precondition (could be quickly checked with a gdb_assert) that it must sort after the last element of the vector? However, I guess that for lazy CU expansion, we'll need the ability to insert blocks in the middle, as we expand more of a CU? In this case, we would perhaps want a way to add all the new blocks, and then do one sort at the end, rather than sorted insertions. I also wish we could avoid having a "set_block" method, because it requires having the blockvector in an invalid state (some blocks slots set to nullptr) while we build it, which could be error prone. The users of the set_block method are buildsym and jit. From what i saw, they could both very well prepare an std::vector with their blocks and std::move that vector into a blockvector constructor. That constructor could assert that the blocks are correctly ordered, or to the sort itself (something that both buildsym and jit do already). buildsym starts with a singly-linked list (m_pending_blocks), moves the blocks to a temporary vector (in end_compunit_symtab_get_static_block), sorts that vector, builds a singly-linked list again, and then traverses that list to insert the blocks into the blockvector. I think that could all be simplified by making m_pending_blocks an std::vector from the start. Hopefully, something like this would make it harder to build a blockvector in an invalid state. Simon