From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8438 invoked by alias); 24 May 2012 17:59:10 -0000 Received: (qmail 8428 invoked by uid 22791); 24 May 2012 17:59:08 -0000 X-SWARE-Spam-Status: No, hits=-3.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KAM_STOCKGEN,KHOP_RCVD_TRUST,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_BJ,TW_FN,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-wg0-f73.google.com (HELO mail-wg0-f73.google.com) (74.125.82.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 May 2012 17:58:55 +0000 Received: by wgbdq12 with SMTP id dq12so2915wgb.0 for ; Thu, 24 May 2012 10:58:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=to:cc:subject:message-id:date:from:x-gm-message-state; bh=IkBswOIIlsL+SeOeQLzlu/kFFPv0yuafXbO1o4WMzc4=; b=HkYhCE0dzaIFDksbNZ7vh3aLO+ZPoMCsY38KFuLf7dOMUrTBCGEduvKne2O0HW+dY+ Kqb9+OGrxTqF8k7V/sXvLUpf2hSs8zjgqfr1SkRXB+DnEK4YsISKUws+gd4geDdiWvUV t5KC6RAetHs0wyMEYNIhSTbKcyLgZQSh9PpoXm8i8prvL8SU7paeRjRQCw0gnovbLXZI 62qthsAK1mYSPZd9RJT5mBeE/eaeTSZf1lCZsYlV5SUinCjehOuDGJGURo1yYZvTAAnO NPcUTBR0TytygOTJxa4XlnKCTj2DSbaWC1zB/shZV2lhxgGi44f4MBIKn1bcuHZoO4Qt pqCA== Received: by 10.14.127.4 with SMTP id c4mr181956eei.3.1337882334054; Thu, 24 May 2012 10:58:54 -0700 (PDT) Received: by 10.14.127.4 with SMTP id c4mr181953eei.3.1337882333982; Thu, 24 May 2012 10:58:53 -0700 (PDT) Received: from hpza10.eem.corp.google.com ([74.125.121.33]) by gmr-mx.google.com with ESMTPS id b16si28395182eeg.3.2012.05.24.10.58.53 (version=TLSv1/SSLv3 cipher=AES128-SHA); Thu, 24 May 2012 10:58:53 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.18.110.129]) by hpza10.eem.corp.google.com (Postfix) with ESMTP id 9B44520017B; Thu, 24 May 2012 10:58:53 -0700 (PDT) Received: by ruffy2.mtv.corp.google.com (Postfix, from userid 67641) id D38381E139C; Thu, 24 May 2012 10:58:52 -0700 (PDT) To: gdb-patches@sourceware.org cc: ratmice@gmail.com Subject: [RFA] massively speed up "info var foo" on large programs Message-Id: <20120524175852.D38381E139C@ruffy2.mtv.corp.google.com> Date: Thu, 24 May 2012 17:59:00 -0000 From: dje@google.com (Doug Evans) X-Gm-Message-State: ALoCoQllFJLSIiQYxSINeCNE28O4RLqZaNSJB1tc0JjPHGy4YNbkqmN/weOP47s/yz3qkz40NaGOCZQX4V5QbkdjTRhhuFwUhiy5DuqgW0k3AtwFPVPhqgoi98CjUK3Fuy3QC8P54LqcoVoQrCujklytx491W/w/9Q== X-IsSubscribed: yes 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 X-SW-Source: 2012-05/txt/msg00942.txt.bz2 Hi. I'm not entirely sure this patch is correct, but it feels correct (*1), and is a massive win. "info var Task" in one large program goes from 350 seconds to 28 seconds. The problem is the two loops over all minsyms of all objfiles. The call to lookup_symbol looks up all those minsyms in all symtabs of all objfiles. Given an early test in the code of where the minsym lives (e.g. for variables the code checks mst_data, mst_bss, mst_file_data, mst_file_bss), it doesn't feel right to look in other objfiles. It's possible there's a good reason though. Alas I can't think of one (I'm probably just blinded by the massive speedup ...). This issue has been looked at before. E.g., http://sourceware.org/ml/gdb-patches/2011-09/msg00461.html (*1): There's a FIXME in the patch that I want to remove prior to check-in, I'm just not sure what the right fix is. For functions: why call both find_pc_symtab and lookup_symbol? The patch leaves the code as is, so I could just change the FIXME into a comment explaining why both calls are made. I'd rather remove the call to find_pc_symtab (in the second minsym loop). An alternative is to remove both find_pc_symtab calls and just call lookup_msymbol_in_objfile for variables and functions, I'm guessing the find_pc_symtab is used in the first minsym loop because it's faster, but I wish that weren't just a guess. [Gotta love comments that explain why things are they way they are. :-)] One thing the patch does is change the test in the first minsym loop to not call find_pc_symtab for variables. I can put the test back, but the call seems unnecessary for variables given that lookup_symbol (or now lookup_msymbol_in_objfile) is also called. The patch gives identical results for the cases I tried (with multi-1000 line output), which raises my confidence that it's correct, but not entirely so. Regression-tested on amd64-linux. What's the right way to fix the FIXME? And ok to check in after that's fixed? 2012-05-23 Doug Evans * symtab.c (lookup_msymbol_in_objfile): New function. (search_symbols): Call it. Index: symtab.c =================================================================== RCS file: /cvs/src/src/gdb/symtab.c,v retrieving revision 1.306 diff -u -p -r1.306 symtab.c --- symtab.c 24 May 2012 02:51:48 -0000 1.306 +++ symtab.c 24 May 2012 04:19:31 -0000 @@ -1559,6 +1559,30 @@ lookup_symbol_aux_symtabs (int block_ind return NULL; } +/* Wrapper around lookup_symbol_aux_objfile for search_symbols. + Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE. */ + +static struct symbol * +lookup_msymbol_in_objfile (struct objfile *objfile, + struct minimal_symbol *msymbol, + domain_enum domain) +{ + const char *name = SYMBOL_LINKAGE_NAME (msymbol); + enum language lang = current_language->la_language; + const char *modified_name; + struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name); + struct symbol *returnval; + + returnval = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK, + modified_name, domain); + if (returnval == NULL) + returnval = lookup_symbol_aux_objfile (objfile, STATIC_BLOCK, + modified_name, domain); + + do_cleanups (cleanup); + return returnval; +} + /* A helper function for lookup_symbol_aux that interfaces with the "quick" symbol table functions. */ @@ -3463,21 +3487,13 @@ search_symbols (char *regexp, enum searc || regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0, NULL, 0) == 0) { - if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))) - { - /* FIXME: carlton/2003-02-04: Given that the - semantics of lookup_symbol keeps on changing - slightly, it would be a nice idea if we had a - function lookup_symbol_minsym that found the - symbol associated to a given minimal symbol (if - any). */ - if (kind == FUNCTIONS_DOMAIN - || lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol), - (struct block *) NULL, - VAR_DOMAIN, 0) - == NULL) - found_misc = 1; - } + /* Note: An important side-effect of these lookup functions + is to expand the symbol table if msymbol is found. */ + if (kind == FUNCTIONS_DOMAIN + ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL + : lookup_msymbol_in_objfile (objfile, msymbol, + VAR_DOMAIN) == NULL) + found_misc = 1; } } } @@ -3570,13 +3586,13 @@ search_symbols (char *regexp, enum searc NULL, 0) == 0) { /* Functions: Look up by address. */ - if (kind != FUNCTIONS_DOMAIN || - (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))) + if (kind != FUNCTIONS_DOMAIN + || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL) { /* Variables/Absolutes: Look up by name. */ - if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol), - (struct block *) NULL, VAR_DOMAIN, 0) - == NULL) + /* FIXME: Why do we also look up fns by name? */ + if (lookup_msymbol_in_objfile (objfile, msymbol, + VAR_DOMAIN) == NULL) { /* match */ psr = (struct symbol_search *)