From patchwork Tue Aug 7 14:57:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 28771 Received: (qmail 80985 invoked by alias); 7 Aug 2018 14:57:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 80975 invoked by uid 89); 7 Aug 2018 14:57:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=awareness, million, yeah, hundred X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Aug 2018 14:57:01 +0000 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A93B2C057FA3 for ; Tue, 7 Aug 2018 14:57:00 +0000 (UTC) Received: from theo.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 77CAB308BDA6 for ; Tue, 7 Aug 2018 14:57:00 +0000 (UTC) Subject: Re: [PATCH 6/8] Use std::unordered_map instead of htab_t. To: gdb-patches@sourceware.org References: <20180503184153.31183-1-keiths@redhat.com> <20180503184153.31183-7-keiths@redhat.com> <3b3521c5-6ea3-58ce-8958-4d4be3d91ca6@redhat.com> <4bbb12c8-927e-aed8-6656-661cc59cf185@redhat.com> From: Keith Seitz Message-ID: <6182299f-ea49-9b68-8104-b797b40c0014@redhat.com> Date: Tue, 7 Aug 2018 07:57:00 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <4bbb12c8-927e-aed8-6656-661cc59cf185@redhat.com> X-IsSubscribed: yes On 07/11/2018 04:21 AM, Pedro Alves wrote: > Yeah, it's not whether or not I _want_ you to revert. Instead, data rules. Ha ha. Indeed, it (data) does rule. I apologize for being so vague... > I really have no awareness of whether that map ends up with a hundred entries > or a hundred million entries, and thus whether the difference ends up > significant. So if after considering that you're not concerned, then > neither am I. I've revisited this topic a bit on the my C++ compile branch, and there is sufficient evidence there to suggest that the type cache could grow quite large -- especially for non-trivial compilations such as compiling a file (or perhaps in the future if/when we support code replacement or other advanced uses of this feature. IIRC, I originally wrote this before the move to C++11 had been proposed. Consequently unique_ptr was not available. Of course, it is now, and it makes sense to me to move to using htab_up instead. [The underlying storage for the error map doesn't really matter. That map seldom has any entries in it.] WDYT of the below patch? Keith Subject: [PATCH] Use unique_ptr for htabs This patch updates the type-conversion caching in C compile to use unique pointers. This patch also removes the on-demand allocation of the symbol error map in favor of initialization, simplifying the code. gdb/ChangeLog YYYY-MM-DD Keith Seitz * compile/compile-internal.h (compile_instance::~compile_instance): Remove calls to htab_delete. : Switch type to htab_up. * compile.c (compile_instance::compile_instance): Initialize htab unique pointers. (compile_instance::get_cached_type, compile_instance::insert_type) (compile_instance::error_symbol_once): Update for unique_ptr. --- gdb/compile/compile-internal.h | 7 ++----- gdb/compile/compile.c | 28 ++++++++++------------------ 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h index 3916f84f6f..c8d2d2f427 100644 --- a/gdb/compile/compile-internal.h +++ b/gdb/compile/compile-internal.h @@ -49,9 +49,6 @@ public: virtual ~compile_instance () { m_gcc_fe->ops->destroy (m_gcc_fe); - htab_delete (m_type_map); - if (m_symbol_err_map != NULL) - htab_delete (m_symbol_err_map); } /* Returns the GCC options to be passed during compilation. */ @@ -148,10 +145,10 @@ protected: std::string m_gcc_target_options; /* Map from gdb types to gcc types. */ - htab_t m_type_map; + htab_up m_type_map; /* Map from gdb symbols to gcc error messages to emit. */ - htab_t m_symbol_err_map; + htab_up m_symbol_err_map; }; /* Define header and footers for different scopes. */ diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 308c82ee54..31ae5970b9 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -129,11 +129,13 @@ del_symbol_error (void *a) compile_instance::compile_instance (struct gcc_base_context *gcc_fe, const char *options) : m_gcc_fe (gcc_fe), m_gcc_target_options (options), - m_symbol_err_map (NULL) + m_type_map (htab_create_alloc (10, hash_type_map_instance, + eq_type_map_instance, + xfree, xcalloc, xfree)), + m_symbol_err_map (htab_create_alloc (10, hash_symbol_error, + eq_symbol_error, del_symbol_error, + xcalloc, xfree)) { - m_type_map = htab_create_alloc (10, hash_type_map_instance, - eq_type_map_instance, - xfree, xcalloc, xfree); } /* See compile-internal.h. */ @@ -144,7 +146,7 @@ compile_instance::get_cached_type (struct type *type, gcc_type &ret) const struct type_map_instance inst, *found; inst.type = type; - found = (struct type_map_instance *) htab_find (m_type_map, &inst); + found = (struct type_map_instance *) htab_find (m_type_map.get (), &inst); if (found != NULL) { ret = found->gcc_type_handle; @@ -164,7 +166,7 @@ compile_instance::insert_type (struct type *type, gcc_type gcc_type) inst.type = type; inst.gcc_type_handle = gcc_type; - slot = htab_find_slot (m_type_map, &inst, INSERT); + slot = htab_find_slot (m_type_map.get (), &inst, INSERT); add = (struct type_map_instance *) *slot; /* The type might have already been inserted in order to handle @@ -189,18 +191,8 @@ compile_instance::insert_symbol_error (const struct symbol *sym, struct symbol_error e; void **slot; - if (m_symbol_err_map == NULL) - { - m_symbol_err_map = htab_create_alloc (10, - hash_symbol_error, - eq_symbol_error, - del_symbol_error, - xcalloc, - xfree); - } - e.sym = sym; - slot = htab_find_slot (m_symbol_err_map, &e, INSERT); + slot = htab_find_slot (m_symbol_err_map.get (), &e, INSERT); if (*slot == NULL) { struct symbol_error *e = XNEW (struct symbol_error); @@ -223,7 +215,7 @@ compile_instance::error_symbol_once (const struct symbol *sym) return; search.sym = sym; - err = (struct symbol_error *) htab_find (m_symbol_err_map, &search); + err = (struct symbol_error *) htab_find (m_symbol_err_map.get (), &search); if (err == NULL || err->message == NULL) return;