From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32689 invoked by alias); 6 Nov 2011 06:31:12 -0000 Received: (qmail 32678 invoked by uid 22791); 6 Nov 2011 06:31:11 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 06 Nov 2011 06:30:56 +0000 Received: from hpaq5.eem.corp.google.com (hpaq5.eem.corp.google.com [172.25.149.5]) by smtp-out.google.com with ESMTP id pA66UtXP006279 for ; Sat, 5 Nov 2011 23:30:55 -0700 Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.18.110.50]) by hpaq5.eem.corp.google.com with ESMTP id pA66Usps007374 for ; Sat, 5 Nov 2011 23:30:54 -0700 Received: by ruffy.mtv.corp.google.com (Postfix, from userid 67641) id B56F6246194; Sat, 5 Nov 2011 23:30:53 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [RFC] Avoid calling gdb_realpath if basenames are different Message-Id: <20111106063053.B56F6246194@ruffy.mtv.corp.google.com> Date: Sun, 06 Nov 2011 06:31:00 -0000 From: dje@google.com (Doug Evans) X-System-Of-Record: true 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: 2011-11/txt/msg00138.txt.bz2 Hi. This patch has been brought up before (by others). E.g., http://sourceware.org/ml/gdb-patches/2010-04/msg00466.html I'm hoping we can get this in now. We're paying a real and significant cost for what is mostly a theoretical concern. [E.g., How often is one source file referred to by the user using a basename that is different than what's recorded in the debug info?] If people are concerned about breaking someone's usage, we could default basenames-may-differ to true in 7.4, with a warning that it will be set to false in 7.5 (or some such). [We could leave the default set to true, especially if someone knew of at least some minimally common usage this would break. I'd hate to otherwise penalize the vast majority of users if not.] I'll write docs,NEWS if the basics of the patch are approved. Comments? 2011-11-05 Doug Evans * dwarf2read.c (dw2_lookup_symtab): Avoid calling gdb_realpath if ! basenames_may_differ. * psymtab.c (lookup_partial_symtab): Ditto. * symtab.c (lookup_partial_symtab): Ditto. * symtab.c (lookup_symtab): Ditto. (basenames_may_differ): New global. (_initialize_symtab): New parameter basenames-may-differ. * symtab.h (basenames_may_differ): Declare. Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.578 diff -u -p -r1.578 dwarf2read.c --- dwarf2read.c 20 Oct 2011 23:13:01 -0000 1.578 +++ dwarf2read.c 6 Nov 2011 06:25:11 -0000 @@ -2444,7 +2444,8 @@ dw2_lookup_symtab (struct objfile *objfi struct symtab **result) { int i; - int check_basename = lbasename (name) == name; + const char *name_basename = lbasename (name); + int check_basename = name_basename == name; struct dwarf2_per_cu_data *base_cu = NULL; dw2_setup (objfile); @@ -2477,6 +2478,12 @@ dw2_lookup_symtab (struct objfile *objfi && FILENAME_CMP (lbasename (this_name), name) == 0) base_cu = per_cu; + /* Before we invoke realpath, which can get expensive when many + files are involved, do a quick comparison of the basenames. */ + if (! basenames_may_differ + && FILENAME_CMP (lbasename (this_name), name_basename) != 0) + continue; + if (full_path != NULL) { const char *this_real_name = dw2_get_real_path (objfile, Index: psymtab.c =================================================================== RCS file: /cvs/src/src/gdb/psymtab.c,v retrieving revision 1.31 diff -u -p -r1.31 psymtab.c --- psymtab.c 28 Oct 2011 17:29:37 -0000 1.31 +++ psymtab.c 6 Nov 2011 06:25:11 -0000 @@ -134,6 +134,7 @@ lookup_partial_symtab (struct objfile *o const char *full_path, const char *real_path) { struct partial_symtab *pst; + const char *name_basename = lbasename (name); ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst) { @@ -142,6 +143,12 @@ lookup_partial_symtab (struct objfile *o return (pst); } + /* Before we invoke realpath, which can get expensive when many + files are involved, do a quick comparison of the basenames. */ + if (! basenames_may_differ + && FILENAME_CMP (name_basename, lbasename (pst->filename)) != 0) + continue; + /* If the user gave us an absolute path, try to find the file in this symtab and use its absolute path. */ if (full_path != NULL) @@ -172,7 +179,7 @@ lookup_partial_symtab (struct objfile *o /* Now, search for a matching tail (only if name doesn't have any dirs). */ - if (lbasename (name) == name) + if (name_basename == name) ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst) { if (FILENAME_CMP (lbasename (pst->filename), name) == 0) Index: symtab.c =================================================================== RCS file: /cvs/src/src/gdb/symtab.c,v retrieving revision 1.285 diff -u -p -r1.285 symtab.c --- symtab.c 29 Oct 2011 07:26:07 -0000 1.285 +++ symtab.c 6 Nov 2011 06:25:11 -0000 @@ -112,6 +112,8 @@ void _initialize_symtab (void); /* */ +int basenames_may_differ = 1; + /* Allow the user to configure the debugger behavior with respect to multiple-choice menus when more than one symbol matches during a symbol lookup. */ @@ -155,6 +157,7 @@ lookup_symtab (const char *name) char *real_path = NULL; char *full_path = NULL; struct cleanup *cleanup; + const char* base_name = lbasename (name); cleanup = make_cleanup (null_cleanup, NULL); @@ -180,6 +183,12 @@ got_symtab: return s; } + /* Before we invoke realpath, which can get expensive when many + files are involved, do a quick comparison of the basenames. */ + if (! basenames_may_differ + && FILENAME_CMP (base_name, lbasename (s->filename)) != 0) + continue; + /* If the user gave us an absolute path, try to find the file in this symtab and use its absolute path. */ @@ -4883,5 +4892,20 @@ Show how the debugger handles ambiguitie Valid values are \"ask\", \"all\", \"cancel\", and the default is \"all\"."), NULL, NULL, &setlist, &showlist); + add_setshow_boolean_cmd ("basenames-may-differ", class_files, + &basenames_may_differ, _("\ +Set whether a source file may have multiple base names."), _("\ +Show whether a source file may have multiple names."), _("\ +When doing file name based lookups, gdb will canonicalize file names\n\ +(e.g., expand symlinks) before comparing them, which is an expensive\n\ +operation.\n\ +If set, gdb cannot assume a file is known by one base name, and thus\n\ +it cannot optimize file name comparisions by skipping the canonicalization\n\ +step if the base names are different.\n\ +If not set, all source files must be known by one base name,\n\ +and gdb will do file name comparisons more efficiently."), + NULL, NULL, + &setlist, &showlist); + observer_attach_executable_changed (symtab_observer_executable_changed); } Index: symtab.h =================================================================== RCS file: /cvs/src/src/gdb/symtab.h,v retrieving revision 1.190 diff -u -p -r1.190 symtab.h --- symtab.h 9 Oct 2011 19:34:18 -0000 1.190 +++ symtab.h 6 Nov 2011 06:25:11 -0000 @@ -1307,4 +1307,6 @@ void fixup_section (struct general_symbo struct objfile *lookup_objfile_from_block (const struct block *block); +extern int basenames_may_differ; + #endif /* !defined(SYMTAB_H) */