From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 347 invoked by alias); 30 May 2013 23:27:38 -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 335 invoked by uid 89); 30 May 2013 23:27:38 -0000 X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,SPF_PASS,TW_XS autolearn=ham version=3.3.1 Received: from mail-gg0-f202.google.com (HELO mail-gg0-f202.google.com) (209.85.161.202) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 30 May 2013 23:27:28 +0000 Received: by mail-gg0-f202.google.com with SMTP id o1so89550ggn.5 for ; Thu, 30 May 2013 16:27:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:mime-version:content-type:content-transfer-encoding:message-id :date:to:cc:subject:x-mailer:x-gm-message-state; bh=to5BP56Pq649WH3qwufCoT406ZtRyig1WDagkM82hao=; b=VLJcHvPYYmJiSXjyhHXoS1NEEz3Md2/TPwtgy1eMs1sam+GSITeBPjTCTUcGTsSSE8 OdHeHTZ9gY2tJK/aO7b8RPPw9J0o1p7v9mkgztWLidI7/UdfH0/zrcTXdXOTBMjdFUyW t7XGw/nCqz3nVbac6ZLJuh81mPzn7A4/yEdzOjv73DASuw5txFNxMzRPgjX+iVG+cYdq JFzRDWkM/LEuhh7Anx99a1xVV0KJBKVr3H6kyXfWPmfPNTaZWuXW5cE3iibveKLFH2hQ WbZS5OwzNmKxnDAGeevXGfzFxCgRaufTp8HZaJtuOmW+4LwW9ZpAlOTrfH0u02v7kLlP QtWw== X-Received: by 10.236.137.176 with SMTP id y36mr5086970yhi.6.1369956446342; Thu, 30 May 2013 16:27:26 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id 10si1416260yhj.6.2013.05.30.16.27.26 for (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Thu, 30 May 2013 16:27:26 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 6E1EF31C19C; Thu, 30 May 2013 16:27:25 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <20903.57436.871210.593441@ruffy.mtv.corp.google.com> Date: Thu, 30 May 2013 23:27:00 -0000 To: Keith Seitz , brobecker@adacore.com, psmith@gnu.org Cc: gdb-patches@sourceware.org Subject: [patch] Improve symbol lookup performance noted in PR 15519 X-Gm-Message-State: ALoCoQlW2fBAtaHpt6qZlrF2icq7Ch1oTV+Zq71Zg9Jg5XLXkmOyYwQMvZiqYp5ql5QyHF2tJWmAiemADFTbblJSD1oR1fpnoBHUiEymf78pEOCVgL35IS7HlQVmF5qAnpRDWncUwrqzoISoCgn+cBy0RyvtLI0lzToRryDwGlsvFAuImyDfi01QsNxbr2J+ynj/UB1r6Yj0/+mAW2pDKk8MOdB7S3dmGw== X-SW-Source: 2013-05/txt/msg01097.txt.bz2 Hi. Here's the patch I intend to check in to "fix" http://sourceware.org/bugzilla/show_bug.cgi?id=15519 [It's not a complete "fix", there's still some performance gains to be had, but I'm leaving that for a separate pass. This gets us >95% of the way there, at least in the benchmarks I've been using, including the one in the PR. Thanks Paul!] No regressions on amd64-linux, with/without Fission. I'll let it sit for a few days in case there are any more comments. I'd also like to commit this to the 7.6 branch. Ok Joel? [I need to rerun the testsuite in that tree before committing there.] 2013-05-30 Doug Evans Keith Seitz PR 15519 * cp-namespace.c (find_symbol_in_baseclass): Call cp_lookup_symbol_in_namespace instead of cp_lookup_symbol_namespace. Remove call to lookup_symbol_static. Call lookup_static_symbol_aux unconditionally. Call check_typedef on base types before accessing them. (cp_lookup_nested_symbol): Fix comment. testsuite/ * gdb.cp/derivation2.cc: New file. * gdb.cp/derivation.cc (main): Call foo2. * gdb.cp/derivation.exp: Add tests for typedefs in another file, and when there's an active block. Index: cp-namespace.c =================================================================== RCS file: /cvs/src/src/gdb/cp-namespace.c,v retrieving revision 1.66 diff -u -p -r1.66 cp-namespace.c --- ./cp-namespace.c 14 Mar 2013 11:13:34 -0000 1.66 +++ ./cp-namespace.c 30 May 2013 22:47:02 -0000 @@ -717,36 +717,34 @@ find_symbol_in_baseclass (struct type *p for (i = 0; i < TYPE_N_BASECLASSES (parent_type); ++i) { size_t len; + struct type *base_type = TYPE_BASECLASS (parent_type, i); const char *base_name = TYPE_BASECLASS_NAME (parent_type, i); if (base_name == NULL) continue; /* Search this particular base class. */ - sym = cp_lookup_symbol_namespace (base_name, name, block, VAR_DOMAIN); + sym = cp_lookup_symbol_in_namespace (base_name, name, block, + VAR_DOMAIN, 0); if (sym != NULL) break; + /* Now search all static file-level symbols. We have to do this + for things like typedefs in the class. And we have to search + all static blocks, even if block != NULL, because there's no + guarantees as to which block it's in. */ len = strlen (base_name) + 2 + strlen (name) + 1; concatenated_name = xrealloc (concatenated_name, len); xsnprintf (concatenated_name, len, "%s::%s", base_name, name); - sym = lookup_symbol_static (concatenated_name, block, VAR_DOMAIN); - - /* If there is currently no BLOCK, e.g., the inferior hasn't yet - been started, then try searching all STATIC_BLOCK symbols in - all objfiles. */ - if (block == NULL) - { - sym = lookup_static_symbol_aux (concatenated_name, VAR_DOMAIN); - if (sym != NULL) - break; - } + sym = lookup_static_symbol_aux (concatenated_name, VAR_DOMAIN); + if (sym != NULL) + break; /* If this class has base classes, search them next. */ - if (TYPE_N_BASECLASSES (TYPE_BASECLASS (parent_type, i)) > 0) + CHECK_TYPEDEF (base_type); + if (TYPE_N_BASECLASSES (base_type) > 0) { - sym = find_symbol_in_baseclass (TYPE_BASECLASS (parent_type, i), - name, block); + sym = find_symbol_in_baseclass (base_type, name, block); if (sym != NULL) break; } @@ -794,8 +792,8 @@ cp_lookup_nested_symbol (struct type *pa if (sym != NULL) return sym; - /* Now search all static file-level symbols. Not strictly - correct, but more useful than an error. We do not try to + /* Now search all static file-level symbols. We have to do this + for things like typedefs in the class. We do not try to guess any imported namespace as even the fully specified namespace search is already not C++ compliant and more assumptions could make it too magic. */ Index: testsuite/gdb.cp/derivation.cc =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/derivation.cc,v retrieving revision 1.5 diff -u -p -r1.5 derivation.cc --- ./testsuite/gdb.cp/derivation.cc 1 Jan 2013 06:33:27 -0000 1.5 +++ ./testsuite/gdb.cp/derivation.cc 30 May 2013 22:47:02 -0000 @@ -16,6 +16,8 @@ along with this program. If not, see . */ +extern void foo2 (); /* from derivation2.cc */ + namespace N { typedef double value_type; struct Base { typedef int value_type; }; @@ -306,9 +308,7 @@ int main(void) N::Derived::value_type d = 1; N::value_type n = 3.0; dobj.doit (); + foo2 (); return 0; } - - - Index: testsuite/gdb.cp/derivation.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/derivation.exp,v retrieving revision 1.24 diff -u -p -r1.24 derivation.exp --- ./testsuite/gdb.cp/derivation.exp 1 Jan 2013 06:33:27 -0000 1.24 +++ ./testsuite/gdb.cp/derivation.exp 30 May 2013 22:47:02 -0000 @@ -32,14 +32,15 @@ if { [skip_cplus_tests] } { continue } load_lib "cp-support.exp" -standard_testfile .cc +standard_testfile derivation.cc derivation2.cc -if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} { +if {[prepare_for_testing $testfile.exp $testfile \ + [list $srcfile $srcfile2] {debug c++}]} { return -1 } # Check inheritance of typedefs. -foreach klass {"A" "D" "E" "F"} { +foreach klass {"A" "D" "E" "F" "A2" "D2"} { gdb_test "ptype ${klass}::value_type" "type = int" gdb_test "whatis ${klass}::value_type" "type = int" gdb_test "p (${klass}::value_type) 0" " = 0" @@ -57,6 +58,13 @@ if ![runto 'marker1'] then { continue } +# Check inheritance of typedefs again, but this time with an active block. +foreach klass {"A" "D" "A2" "D2"} { + gdb_test "ptype ${klass}::value_type" "type = int" + gdb_test "whatis ${klass}::value_type" "type = int" + gdb_test "p (${klass}::value_type) 0" " = 0" +} + gdb_test "up" ".*main.*" "up from marker1" # Print class types and values. Index: testsuite/gdb.cp/derivation2.cc =================================================================== RCS file: testsuite/gdb.cp/derivation2.cc diff -N testsuite/gdb.cp/derivation2.cc --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ ./testsuite/gdb.cp/derivation2.cc 30 May 2013 22:47:02 -0000 @@ -0,0 +1,49 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + */ + +/* A copy of some classes in derivation.cc so that we can test symbol lookup + in other CUs. */ + +class A2 { +public: + typedef int value_type; + value_type a; + + A2() + { + a=1; + } +}; + +class D2 : public A2 { +public: + value_type d; + + D2() + { + d=7; + } +}; + +void +foo2 () +{ + D2 d2_instance; + d2_instance.a = 42; + d2_instance.d = 43; +}