FYI: fix some performance bugs with .gdb_index

FYI: fix some performance bugs with .gdb_index

[Tom Tromey]

I'm checking this in.

PR 13498 concerns gdb performance problems with DWARF 4 and also with
.gdb_index.

This patch fixes the index-related problems.

The .so I used for the test (libooxlo.so from LibreOffice) has 262 CUs
and 16597 TUs.  The bug reporter indicated that the problem was that we
were redundantly revisiting file names, and some local profiling
confirmed this.

The fix is to keep track of which file names we have already visited.
This works because multiple TUs will share the same file name table.

I don't think this will drastically impact smaller test cases.
The overhead of the hash table isn't very big.

Built and regtested on x86-64 Fedora 16.

Tom

2012-02-20  Tom Tromey  <tromey@redhat.com>

	PR gdb/13498:
	* dwarf2read.c (dw2_expand_symtabs_matching): Only visit a
	particular set of file names once.
	(dw2_map_symbol_filenames): Likewise.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0576def..5892a64 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2707,32 +2707,63 @@ dw2_expand_symtabs_matching
   index = dwarf2_per_objfile->index_table;
 
   if (file_matcher != NULL)
-    for (i = 0; i < (dwarf2_per_objfile->n_comp_units
-		     + dwarf2_per_objfile->n_type_units); ++i)
-      {
-	int j;
-	struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
-	struct quick_file_names *file_data;
+    {
+      struct cleanup *cleanup;
+      htab_t visited_found, visited_not_found;
 
-	per_cu->v.quick->mark = 0;
+      visited_found = htab_create_alloc (10,
+					 htab_hash_pointer, htab_eq_pointer,
+					 NULL, xcalloc, xfree);
+      cleanup = make_cleanup_htab_delete (visited_found);
+      visited_not_found = htab_create_alloc (10,
+					     htab_hash_pointer, htab_eq_pointer,
+					     NULL, xcalloc, xfree);
+      make_cleanup_htab_delete (visited_not_found);
 
-	/* We only need to look at symtabs not already expanded.  */
-	if (per_cu->v.quick->symtab)
-	  continue;
+      for (i = 0; i < (dwarf2_per_objfile->n_comp_units
+		       + dwarf2_per_objfile->n_type_units); ++i)
+	{
+	  int j;
+	  struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
+	  struct quick_file_names *file_data;
+	  void **slot;
 
-	file_data = dw2_get_file_names (objfile, per_cu);
-	if (file_data == NULL)
-	  continue;
+	  per_cu->v.quick->mark = 0;
 
-	for (j = 0; j < file_data->num_file_names; ++j)
-	  {
-	    if (file_matcher (file_data->file_names[j], data))
-	      {
-		per_cu->v.quick->mark = 1;
-		break;
-	      }
-	  }
-      }
+	  /* We only need to look at symtabs not already expanded.  */
+	  if (per_cu->v.quick->symtab)
+	    continue;
+
+	  file_data = dw2_get_file_names (objfile, per_cu);
+	  if (file_data == NULL)
+	    continue;
+
+	  if (htab_find (visited_not_found, file_data) != NULL)
+	    continue;
+	  else if (htab_find (visited_found, file_data) != NULL)
+	    {
+	      per_cu->v.quick->mark = 1;
+	      continue;
+	    }
+
+	  for (j = 0; j < file_data->num_file_names; ++j)
+	    {
+	      if (file_matcher (file_data->file_names[j], data))
+		{
+		  per_cu->v.quick->mark = 1;
+		  break;
+		}
+	    }
+
+	  slot = htab_find_slot (per_cu->v.quick->mark
+				 ? visited_found
+				 : visited_not_found,
+				 file_data, INSERT);
+	  *slot = file_data;
+	}
+
+      do_cleanups (cleanup);
+    }
 
   for (iter = 0; iter < index->symbol_table_slots; ++iter)
     {
@@ -2794,15 +2825,35 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 			  void *data, int need_fullname)
 {
   int i;
+  struct cleanup *cleanup;
+  htab_t visited = htab_create_alloc (10, htab_hash_pointer, htab_eq_pointer,
+				      NULL, xcalloc, xfree);
 
+  cleanup = make_cleanup_htab_delete (visited);
   dw2_setup (objfile);
 
+  /* We can ignore file names coming from already-expanded CUs.  */
+  for (i = 0; i < (dwarf2_per_objfile->n_comp_units
+		   + dwarf2_per_objfile->n_type_units); ++i)
+    {
+      struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
+
+      if (per_cu->v.quick->symtab)
+	{
+	  void **slot = htab_find_slot (visited, per_cu->v.quick->file_names,
+					INSERT);
+
+	  *slot = per_cu->v.quick->file_names;
+	}
+    }
+
   for (i = 0; i < (dwarf2_per_objfile->n_comp_units
 		   + dwarf2_per_objfile->n_type_units); ++i)
     {
       int j;
       struct dwarf2_per_cu_data *per_cu = dw2_get_cu (i);
       struct quick_file_names *file_data;
+      void **slot;
 
       /* We only need to look at symtabs not already expanded.  */
       if (per_cu->v.quick->symtab)
@@ -2812,6 +2863,14 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
       if (file_data == NULL)
 	continue;
 
+      slot = htab_find_slot (visited, file_data, INSERT);
+      if (*slot)
+	{
+	  /* Already visited.  */
+	  continue;
+	}
+      *slot = file_data;
+
       for (j = 0; j < file_data->num_file_names; ++j)
 	{
 	  const char *this_real_name;
@@ -2823,6 +2882,8 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 	  (*fun) (file_data->file_names[j], this_real_name, data);
 	}
     }
+
+  do_cleanups (cleanup);
 }
 
 static int

Re: FYI: fix some performance bugs with .gdb_index

[Jan Kratochvil]

On Mon, 20 Feb 2012 21:42:28 +0100, Tom Tromey wrote:
> +      visited_found = htab_create_alloc (10,
> +					 htab_hash_pointer, htab_eq_pointer,
> +					 NULL, xcalloc, xfree);
[...]
> +      visited_not_found = htab_create_alloc (10,
> +					     htab_hash_pointer, htab_eq_pointer,
> +					     NULL, xcalloc, xfree);
[...]
> +  htab_t visited = htab_create_alloc (10, htab_hash_pointer, htab_eq_pointer,
> +				      NULL, xcalloc, xfree);

Just wouldn't be obstack + hashtab_obstack_allocate more appropriate here?


Thanks,
Jan

Re: FYI: fix some performance bugs with .gdb_index

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> On Mon, 20 Feb 2012 21:42:28 +0100, Tom Tromey wrote:
>> +      visited_found = htab_create_alloc (10,
>> +					 htab_hash_pointer, htab_eq_pointer,
>> +					 NULL, xcalloc, xfree);
Jan> [...]
>> +      visited_not_found = htab_create_alloc (10,
>> +					     htab_hash_pointer, htab_eq_pointer,
>> +					     NULL, xcalloc, xfree);
Jan> [...]
>> +  htab_t visited = htab_create_alloc (10, htab_hash_pointer, htab_eq_pointer,
>> +				      NULL, xcalloc, xfree);

Jan> Just wouldn't be obstack + hashtab_obstack_allocate more
Jan> appropriate here?

I don't think it would make a very big difference.

Tom