[RFA] performance improvement of lookup_partial_symtab

[Andrew Cagney]

> Hello,
> 
> Debugging an executable build with more than 3000 source files, it
> can take several seconds to GDB to output some source file information.
> A reproducer can be generated with the following script (It generated an
> Ada program):

The technical side of this is symtab so I'll avoid that.  On the 
performance side  ....

I think the commentary (in both places) needs to include an example that 
can be reproduced in C/C++.  I also think the commentary needs to 
provide hard numbers supporting the change.  Simple things like how many 
system calls, searches, and for a given system the overall performance 
effect.  Does it, for instance, affect the far more common ``l foo.c:42' 
case?

It is really important that the commentary that goes with 
micro-optimizations such as this are completly standalone - future 
developers can easily reproduce the problem (reason for C/C++ and not 
Ada) and re-confirm that it sill exists.  If this isn't done the 
micro-optimization pass into folk law - can't remove this because, well 
`just because' :-(

However, before that, wait for a comment from the symtab maintainers.

Andrew


> rm main.adb
> rm -rf many_files
> mkdir many_files
> for i in 0 1 2 3 ; do
>    for j in 0 1 2 3 4 5 6 7 8 9 ; do
>       for k in 0 1 2 3 4 5 6 7 8 9 ; do
>          for l in 0 1 2 3 4 5 6 7 8 9 ; do
> echo "with Pack_${i}${j}${k}${l}_Test;" >> main.adb
> echo "package Pack_${i}$j${k}${l}_Test is" \
> 	>> many_files/pack_${i}${j}${k}${l}_test.ads
> echo "   pragma Pure;" \
> 	>> many_files/pack_${i}${j}${k}${l}_test.ads
> echo "   Var_${i}$j${k}${l}_Test : constant String := \"${i}${j}${k}${l}\";" \
> 	>> many_files/pack_${i}${j}${k}${l}_test.ads
> echo "end Pack_${i}$j${k}${l}_Test;" \
> 	>> many_files/pack_${i}${j}${k}${l}_test.ads
>          done
>       done
>    done
> done
> echo "procedure Main is" >> main.adb
> echo "begin" >> main.adb
> echo "   null;" >> main.adb
> echo "end Main;" >> main.adb
> 
> ---
> 
> Compiling with GNAT 3.16a, and debugging with gdb 5.3:
> 
> 
> (gdb) l /home/guitton/bug/C411-003/many_files/pack_0001_test.ads:2
> 
> [after several seconds:]
> 
> 1       package Pack_0001_Test is
> 2          pragma Pure;
> 3          Var_0001_Test : constant String := "0001";
> 4       end Pack_0001_Test;
> (gdb)
> 
> The culprit is lookup_partial_symtab, that may do a call
> to source_full_path_of for every file referenced. I suggest trying
> to avoid as much as possible this function call by doing some preliminary
> testing: if the basenames are not equal, no need to fetch the
> fullname and make the fullname comparison. With this modification
> GDB reacts almost instantly. 
> 
> I have tested this patch against the testsuite on linux; no regression
> appeared. There seems to be no test using an absolute path, I can
> also propose one.
> 
> Joel Brobecker volunteered to commit this path if/when it is approved.
> 
> 
> 2003-05-26  J. Guitton <guitton@gnat.com>
> 
>         * symtab.c (lookup_partial_symbol): To improve the performance,
>         try to avoid as much as possible to call source_full_path_of,
> 	by doing some preliminary testing.
> 
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.109
> diff -c -r1.109 symtab.c
> *** symtab.c	20 May 2003 01:55:17 -0000	1.109
> --- symtab.c	10 Jun 2003 13:22:43 -0000
> ***************
> *** 273,280 ****
>         }
>   
>       /* 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)
>         {
>   	if (pst->fullname == NULL)
>   	  source_full_path_of (pst->filename, &pst->fullname);
> --- 273,283 ----
>         }
>   
>       /* If the user gave us an absolute path, try to find the file in
> !        this symtab and use its absolute path.  However, if the
> !        base names are different, the two files are different and there
> !        is no need to do a full comparison.  */
> !     if (full_path != NULL &&
> ! 	strcmp (lbasename (full_path), lbasename (pst->filename)) == 0)

See the GNU coding standard.  The ``&&'' goes at the start of the line.

>         {
>   	if (pst->fullname == NULL)
>   	  source_full_path_of (pst->filename, &pst->fullname);
> ***************
> *** 285,291 ****
>   	  }
>         }
>   
> !     if (real_path != NULL)
>         {
>           char *rp = NULL;
>   	if (pst->fullname == NULL)
> --- 288,295 ----
>   	  }
>         }
>   
> !     if (real_path != NULL &&
> ! 	strcmp (lbasename (real_path), lbasename (pst->filename)) == 0)
>         {
>           char *rp = NULL;
>   	if (pst->fullname == NULL)
>