From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38592 invoked by alias); 30 Jan 2018 11:39:16 -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 38582 invoked by uid 89); 30 Jan 2018 11:39:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=trading 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, 30 Jan 2018 11:39:15 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E4DDF80464; Tue, 30 Jan 2018 11:39:13 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 04BA45C3FD; Tue, 30 Jan 2018 11:39:12 +0000 (UTC) Subject: Re: [PATCH 4/7] Class-fy partial_die_info To: Simon Marchi , Yao Qi , gdb-patches@sourceware.org References: <1516873114-7449-1-git-send-email-yao.qi@linaro.org> <1516873114-7449-5-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: <3655ae02-6fe5-de74-1e4d-f396b200f226@redhat.com> Date: Tue, 30 Jan 2018 11:39:00 -0000 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: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-01/txt/msg00625.txt.bz2 On 01/29/2018 01:15 AM, Simon Marchi wrote: > From what I understand, the only reason to have that private constructor is > to construct a temporary partial_die_info object used to search in the htab, > is that right? If so, converting that htab_t to an std::unordered_map would > remove the need for all this, since you don't need to construct an object > to search it. See the diff below that applies on top of this patch. > > It's not thoroughly tested and I am not sure of the validity of the > per_cu->cu->partial_dies.empty () call in find_partial_die, but I think it > should work. Plus, it adds some type-safety, which I am a big fan of. > > But otherwise, the patch is fine with me. Careful here. This could do with some benchmarking. The DWARF reading code is performance (both timing and memory) sensitive. This is trading an open addressing hash table (htab_t), for a node-based closed addressing hash table. The keys/elements in the map are small so I'd expect this to make a difference. Also, this is trading a in-principle cache-friendly obstack allocation scheme for the standard new allocator. Thanks, Pedro Alves