From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6402 invoked by alias); 12 Mar 2004 23:45:52 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6392 invoked from network); 12 Mar 2004 23:45:51 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 12 Mar 2004 23:45:51 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i2CNjp07011747 for ; Fri, 12 Mar 2004 18:45:51 -0500 Received: from zenia.home.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i2CNjm822530; Fri, 12 Mar 2004 18:45:49 -0500 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com, Elena Zannoni Subject: Re: [RFA/dwarf] Don't process types multiple times References: <20040225044206.GA23242@nevyn.them.org> From: Jim Blandy Date: Fri, 12 Mar 2004 23:45:00 -0000 In-Reply-To: <20040225044206.GA23242@nevyn.them.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2004-03.o/txt/msg00291.txt Daniel Jacobowitz writes: > This independent patch is a performance and correctness fix for full symbol > processing. My description from the intercu branch posting: > > There are two ways that we can process a general DIE: process_die or > read_type_die. Children of particular DIE types may be processed directly, > but these are the only major dispatch points. It's interesting to notice > that almost everything called from read_type_die starts with "if (die->type) > return": everything but enumeration types and aggregate types, in fact. > This means that if the first reference to an enumeration or aggregate type > is a DW_AT_type or DW_AT_containing_type in a DIE numerically before the > type's DIE, we'll end up calling new_symbol for it twice. > > Fixing this saves about 8% memory and 4% time from gdb -readnow libc.so.6, > a lot of duplicate entries on maint print symbols which I vaguely recall > being confused about but never investigated, and some serious problems for > inter-CU support. Without this, types could be added to the wrong symbol > table. > > Tested i686-pc-linux-gnu, no regressions. OK to commit? So the essential idea here is to make sure that we only create symbols when we reach them via the main traversal, not when we happen to reach them by following references with die_type. The current code relies on the existing read_structure_scope and read_enumeration functions to both read the type and create the symbol, so it can't distinguish between the two cases. Your patch creates a separate function for each role, and ensures that the symbol creation functions are only called from the main traversal. Is that right? If so, please commit, to trunk and branch. One suggestion: for the aggregate types, it seems like we're using "read_structure_scope" to mean "create the type object, but don't call new_symbol", and "process_structure_scope" to mean "go ahead and create the new symbols". But for the enumerations, the two corresponding functions are called "read_enumeration_type" and "read_enumeration_scope". Their functions are parallel, but the names don't reflect it. So how about: don't make sym make sym struct: read_structure_scope process_structure_scope enum: read_enumeration_scope process_enumeration_scope or: struct: read_structure_type process_structure_scope enum: read_enumeration_type process_enumeration_scope From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6402 invoked by alias); 12 Mar 2004 23:45:52 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6392 invoked from network); 12 Mar 2004 23:45:51 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 12 Mar 2004 23:45:51 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i2CNjp07011747 for ; Fri, 12 Mar 2004 18:45:51 -0500 Received: from zenia.home.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i2CNjm822530; Fri, 12 Mar 2004 18:45:49 -0500 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com, Elena Zannoni Subject: Re: [RFA/dwarf] Don't process types multiple times References: <20040225044206.GA23242@nevyn.them.org> From: Jim Blandy Date: Fri, 19 Mar 2004 00:09:00 -0000 In-Reply-To: <20040225044206.GA23242@nevyn.them.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2004-03/txt/msg00291.txt.bz2 Message-ID: <20040319000900.uCOBdMMdLbQqY3fvZ7O7BKgsJg022qehy0M7QS809Ak@z> Daniel Jacobowitz writes: > This independent patch is a performance and correctness fix for full symbol > processing. My description from the intercu branch posting: > > There are two ways that we can process a general DIE: process_die or > read_type_die. Children of particular DIE types may be processed directly, > but these are the only major dispatch points. It's interesting to notice > that almost everything called from read_type_die starts with "if (die->type) > return": everything but enumeration types and aggregate types, in fact. > This means that if the first reference to an enumeration or aggregate type > is a DW_AT_type or DW_AT_containing_type in a DIE numerically before the > type's DIE, we'll end up calling new_symbol for it twice. > > Fixing this saves about 8% memory and 4% time from gdb -readnow libc.so.6, > a lot of duplicate entries on maint print symbols which I vaguely recall > being confused about but never investigated, and some serious problems for > inter-CU support. Without this, types could be added to the wrong symbol > table. > > Tested i686-pc-linux-gnu, no regressions. OK to commit? So the essential idea here is to make sure that we only create symbols when we reach them via the main traversal, not when we happen to reach them by following references with die_type. The current code relies on the existing read_structure_scope and read_enumeration functions to both read the type and create the symbol, so it can't distinguish between the two cases. Your patch creates a separate function for each role, and ensures that the symbol creation functions are only called from the main traversal. Is that right? If so, please commit, to trunk and branch. One suggestion: for the aggregate types, it seems like we're using "read_structure_scope" to mean "create the type object, but don't call new_symbol", and "process_structure_scope" to mean "go ahead and create the new symbols". But for the enumerations, the two corresponding functions are called "read_enumeration_type" and "read_enumeration_scope". Their functions are parallel, but the names don't reflect it. So how about: don't make sym make sym struct: read_structure_scope process_structure_scope enum: read_enumeration_scope process_enumeration_scope or: struct: read_structure_type process_structure_scope enum: read_enumeration_type process_enumeration_scope