This is the mail archive of the
mailing list for the GDB project.
path comparison speedups
- From: Doug Evans <dje at google dot com>
- To: Tom Tromey <tromey at redhat dot com>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Sat, 29 Oct 2011 15:16:39 -0700
- Subject: path comparison speedups
I just noticed that dw2_get_real_path (which dates back to
dwarf2read.c cvs revision 1.414 from Tom, then named
dw2_require_full_path) caches the results of gdb_realpath. This got
me digging deeper.
There's two things I'd like to address with a patch to speed file name
- check basename before trying to match the full path
- decide if we *can* cache full/real-path lookups, how that's done
(e.g. source.c:symtab_to_fullname says "don't cache" but dwarf2read.c
caches them), and clean this up
source.c:symtab_to_fullname stopped caching lookups here:
and ultimately got checked in in cvs revision 1.51 in June 2004.
I read the thread for this patch and I couldn't find any discussion on
this point (other than the patch submitter's contention that this is
how gdb should work).
For reference sake, gdb 6.1.1 cached lookups, but it also had
forget_cached_source_info, which we still have today and has even been
extended so that, for example, dwarf2read.c can flush its cache of
So why *does* this comment exist in source.c?
/* Don't check s->fullname here, the file could have been
deleted/moved/..., look for it again. */
Especially since forget_cached_source_info_for_objfile exists and
flushes s->fullname ...
[If we need to add an explicit command that flushes the cache to
handle cases not already handled by forget_cached_foo, that's fine
Before I spend too much more time on this patch, I'd like to get some
feedback that deleting this comment, and having symtab_to_fullname use
the cached path is ok. Am I missing something?
I'd also like to get feedback that it's ok to first compare basenames
before doing realpath lookups and then comparing the full name.
Appended is a snippet that implements this, not necessarily to be
committed as is, just provided for illustration's sake.
The only time this won't work, I think, is if the user wants to
reference the file by a different basename than is used in the debug
info. Seems exceedingly rare. I'd be happy with an option,
defaulting to the common case, that says whether gdb has to handle the
rare case. We're paying a significant performance penalty to handle a
rare (theoretical?) case.
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 29 Oct 2011 21:11:23 -0000
@@ -155,6 +155,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);
@@ -174,12 +175,18 @@ got_symtab:
ALL_SYMTABS (objfile, s)
+ const char * s_base_name;
if (FILENAME_CMP (name, s->filename) == 0)
+ s_base_name = lbasename (s->filename);
+ if (FILENAME_CMP (base_name, s_base_name) != 0)
/* If the user gave us an absolute path, try to find the file in
this symtab and use its absolute path. */