From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by sourceware.org (Postfix) with ESMTPS id BEC333896C26 for ; Mon, 29 Nov 2021 09:00:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BEC333896C26 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org Received: (Authenticated sender: dodji@seketeli.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 6B3AEFF81C; Mon, 29 Nov 2021 09:00:17 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 7FA255802BD; Mon, 29 Nov 2021 10:00:16 +0100 (CET) From: Dodji Seketeli To: Matthias Maennich Cc: libabigail@sourceware.org, gprocida@google.com, kernel-team@android.com Subject: Re: [PATCH] XML writer: adjust tracking of emitted declarations Organization: Me, myself and I References: <20211124154411.2434715-1-maennich@google.com> X-Operating-System: Fedora 36 X-URL: http://www.seketeli.net/~dodji Date: Mon, 29 Nov 2021 10:00:16 +0100 In-Reply-To: <20211124154411.2434715-1-maennich@google.com> (Matthias Maennich's message of "Wed, 24 Nov 2021 15:44:12 +0000") Message-ID: <87ilwbs5en.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Nov 2021 09:00:21 -0000 Hello, Matthias Maennich a =C3=A9crit: [...] > decl_is_emitted(decl_base_sptr& decl) const > { > - if (is_type(decl)) > - return false; > - Rather than dropping this test, I have turned it into an assert. [...] > Replace the std::unordered_map used to track emitted declarations with a > std::unordered_set as the map only ever held "true". The container > itself does not need to be marked mutable because record_decl_as_emitted > is called in a non-const context and can itself be made non-const. In > addition, the method decl_is_emitted calls a helper which no longer used > anywhere else and can be inlined. The remaining two methods are always > called on non-type declarations, so the test that existed in > decl_is_emitted can be dropped. > > * abg-writer.cc (write_context): Replace mutable > m_emitted_decls_map with plain m_emitted_decls_set. > (decl_name_is_emitted): Inlined into decl_is_emitted; dropped. > (decl_is_emitted): Drop is_type check and inline > decl_name_is_emitted. Look up in set instead of map. > (record_decl_as_emitted): Make non-const. Insert into set > instead of map. > > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich Applied the amended version to master. Thanks! I am attaching the committed patch below. Cheers, >From ce38bd24cd20561c7ebb12e615ba9457feab9e8f Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Wed, 24 Nov 2021 15:44:12 +0000 Subject: [PATCH] XML writer: adjust tracking of emitted declarations Replace the std::unordered_map used to track emitted declarations with a std::unordered_set as the map only ever held "true". The container itself does not need to be marked mutable because record_decl_as_emitted is called in a non-const context and can itself be made non-const. In addition, the method decl_is_emitted calls a helper which no longer used anywhere else and can be inlined. The remaining two methods are always called on non-type declarations, so the test that existed in decl_is_emitted can be dropped. * abg-writer.cc (write_context): Replace mutable m_emitted_decls_map with plain m_emitted_decls_set. (decl_name_is_emitted): Inlined into decl_is_emitted; dropped. (decl_is_emitted): Turn the is_type check into an assert and inline decl_name_is_emitted. Look up in set instead of map. (record_decl_as_emitted): Make non-const. Insert into set instead of map. Reviewed-by: Giuliano Procida Signed-off-by: Matthias Maennich Signed-off-by: Dodji Seketeli --- src/abg-writer.cc | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 693f99c6..21b79e88 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -168,9 +168,7 @@ class write_context class_tmpl_shared_ptr_map m_class_tmpl_id_map; string_elf_symbol_sptr_map_type m_fun_symbol_map; string_elf_symbol_sptr_map_type m_var_symbol_map; - mutable unordered_map m_emitted_decls_map; + unordered_set m_emitted_decls_set; =20 write_context(); =20 @@ -747,17 +745,6 @@ public: type_is_emitted(const type_base_sptr& t) const {return type_is_emitted(t.get());} =20 - /// Test if the name of a given decl has been written out to the XML - /// output. - /// - /// @param the decl to consider. - /// - /// @return true if the decl has already been emitted, false - /// otherwise. - bool - decl_name_is_emitted(const interned_string& name) const - {return m_emitted_decls_map.find(name) !=3D m_emitted_decls_map.end();} - /// Test if a given decl has been written out to the XML output. /// /// @param the decl to consider. @@ -767,13 +754,10 @@ public: bool decl_is_emitted(decl_base_sptr& decl) const { - if (is_type(decl)) - return false; - + ABG_ASSERT(!is_type(decl)); string repr =3D get_pretty_representation(decl, true); interned_string irepr =3D decl->get_environment()->intern(repr); - bool is_emitted =3D decl_name_is_emitted(irepr); - return is_emitted; + return m_emitted_decls_set.find(irepr) !=3D m_emitted_decls_set.end(); } =20 /// Record a declaration-only class as being emitted. @@ -829,11 +813,11 @@ public: /// /// @param decl the decl to consider. void - record_decl_as_emitted(const decl_base_sptr &decl)const + record_decl_as_emitted(const decl_base_sptr& decl) { string repr =3D get_pretty_representation(decl, true); interned_string irepr =3D decl->get_environment()->intern(repr); - m_emitted_decls_map[irepr] =3D true; + m_emitted_decls_set.insert(irepr); } =20 /// Get the set of types that have been emitted. --=20 2.33.1 --=20 Dodji