This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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
comparison up:
- 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
realpath lookups.

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
with me.]

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.

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    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)
        do_cleanups (cleanup);
        return s;

+    s_base_name = lbasename (s->filename);
+    if (FILENAME_CMP (base_name, s_base_name) != 0)
+      continue;
     /* If the user gave us an absolute path, try to find the file in
        this symtab and use its absolute path.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]