From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7888 invoked by alias); 3 May 2011 15:53:39 -0000 Received: (qmail 7877 invoked by uid 22791); 3 May 2011 15:53:38 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor.suse.de (HELO mx1.suse.de) (195.135.220.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 03 May 2011 15:53:24 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id BF7D99428F; Tue, 3 May 2011 17:53:22 +0200 (CEST) Date: Tue, 03 May 2011 15:53:00 -0000 From: Richard Guenther To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Subject: Re: Move cgraph_node_set and varpool_node_set out of GGC and make them use pointer_map In-Reply-To: <20110503153337.GA18705@kam.mff.cuni.cz> Message-ID: References: <20110503153337.GA18705@kam.mff.cuni.cz> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-05/txt/msg00192.txt.bz2 On Tue, 3 May 2011, Jan Hubicka wrote: > Hi, > I always considered the cgrpah_node_set/varpool_node_set to be overengineered > but they also turned out to be quite ineffective since we do quite a lot of > queries into them during stremaing out. > > This patch moves them to pointer_map, like I did for streamer cache. While > doing so I needed to get the structure out of GGC memory since pointer_map is > not ggc firendly. This is not a deal at all, because the sets can only point to > live cgraph/varpool entries anyway. Pointing to removed ones would lead to > spectacular failures in any case. > > Bootstrapped/regtested x86_64-linux, OK? Umm, I wonder why ... > + cgraph_node_set_add (cgraph_node_set set, struct cgraph_node *node) > + { > + void **slot; > + > + slot = pointer_map_insert (set->map, node); > + > + if (*slot) > + { > + int index = (size_t) *slot - 1; > + gcc_checking_assert ((VEC_index (cgraph_node_ptr, set->nodes, index) > + == node)); > + return; > + } > + > + *slot = (void *)(size_t) (VEC_length (cgraph_node_ptr, set->nodes) + 1); > + > + /* Insert into node vector. */ > + VEC_safe_push (cgraph_node_ptr, heap, set->nodes, node); We have both a vector and a pointer-map. Why not simply use a pointer-map only?! I see this may need more re-structuring, eventually adding an iterator interface to pointer-sets/maps (which would be nice anyway). It also makes me think again that why do we have both a cgraph and a varpool set ... at least when you now deal with a non-GC data structure you could as well use a vector of void * and provide macros doing the casting for you, unifying the implementation itself. Well, I suppose most of that can be done as a followup (when eventually the symtab arrives and varpool and cgraph nodes merge anyway). Thus, ok for now. Thanks, Richard.