Re: [patch 2/2] Speed up JIT support

[Paul Pluzhnikov <ppluzhnikov@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

On Fri, Feb 4, 2011 at 12:32 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

>> Maybe the cache could point to the symbols themselves.  I think that
>> would fix the problem and be both safe and efficient.
>
> Ah, that also nicely takes care of the problematic use of -1 as invalid
> core address -- we *know* that 0 and -1 are *not* valid symbol values.

Here is the updated patch (applies on top of the [1/2] cosmetic patch).
Re-tested on linux/x86_64, no regressions.

Thanks,
-- 
Paul Pluzhnikov


2010-02-04  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* breakpoint.c (longjmp_names): New variable.
	(struct breakpoint_objfile_data): New type.
	(breakpoint_objfile_key): New variable.
	(msym_not_found): New variable.
	(msym_not_found_p): New predicate.
	(get_breakpoint_objfile_data): New function.
	(create_overlay_event_breakpoint): Check per-objfile cache for
	symbols first.
	(create_longjmp_master_breakpoint): Likewise.
	(create_std_terminate_master_breakpoint): Likewise.
	(create_exception_master_breakpoint): Likewise.
	(_initialize_breakpoint): Register per-objfile data key.

[-- Attachment #2: gdb-jit-speedup-20110204a.txt --]
[-- Type: text/plain, Size: 8708 bytes --]

--- breakpoint.c.orig	2011-02-04 14:11:34.000000000 -0800
+++ breakpoint.c	2011-02-04 16:11:12.000000000 -0800
@@ -2217,6 +2217,59 @@ create_internal_breakpoint (struct gdbar
   return b;
 }
 
+static const char *const longjmp_names[] =
+  {
+    "longjmp", "_longjmp", "siglongjmp", "_siglongjmp"
+  };
+#define NUM_LONGJMP_NAMES ARRAY_SIZE(longjmp_names)
+
+/* Per-objfile data private to breakpoint.c  */
+struct breakpoint_objfile_data
+{
+  /* minimal symbol for "_ovly_debug_event" (if any)  */
+  struct minimal_symbol *overlay_msym;
+
+  /* minimal symbol(s) for "longjmp", "siglongjmp", etc. (if any)  */
+  struct minimal_symbol *longjmp_msym[NUM_LONGJMP_NAMES];
+
+  /* minimal symbol for "std::terminate()" (if any)  */
+  struct minimal_symbol *terminate_msym;
+
+  /* minimal symbol for "_Unwind_DebugHook" (if any)  */
+  struct minimal_symbol *exception_msym;
+};
+
+static const struct objfile_data *breakpoint_objfile_key;
+
+/* minimal symbol not found sentinel.  */
+static struct minimal_symbol msym_not_found;
+
+static int
+msym_not_found_p (const struct minimal_symbol *msym)
+{
+  return msym == &msym_not_found;
+}
+
+/* Return per-objfile data needed by breakpoint.c.
+   Allocate the data if necessary.  */
+
+static struct breakpoint_objfile_data *
+get_breakpoint_objfile_data (struct objfile *objfile)
+{
+  struct breakpoint_objfile_data *bp_objfile_data;
+
+  bp_objfile_data = objfile_data (objfile, breakpoint_objfile_key);
+  if (bp_objfile_data == NULL)
+    {
+      bp_objfile_data = obstack_alloc (&objfile->objfile_obstack,
+				       sizeof (*bp_objfile_data));
+
+      memset (bp_objfile_data, 0, sizeof (*bp_objfile_data));
+      set_objfile_data (objfile, breakpoint_objfile_key, bp_objfile_data);
+    }
+  return bp_objfile_data;
+}
+
 static void
 create_overlay_event_breakpoint (void)
 {
@@ -2226,14 +2279,30 @@ create_overlay_event_breakpoint (void)
   ALL_OBJFILES (objfile)
     {
       struct breakpoint *b;
-      struct minimal_symbol *m;
+      struct breakpoint_objfile_data *bp_objfile_data;
+      CORE_ADDR addr;
 
-      m = lookup_minimal_symbol_text (func_name, objfile);
-      if (m == NULL)
-        continue;
+      bp_objfile_data = get_breakpoint_objfile_data (objfile);
+
+      if (msym_not_found_p (bp_objfile_data->overlay_msym))
+	continue;
+
+      if (bp_objfile_data->overlay_msym == NULL)
+	{
+	  struct minimal_symbol *m;
+
+	  m = lookup_minimal_symbol_text (func_name, objfile);
+	  if (m == NULL)
+	    {
+	      /* Avoid future lookups in this objfile.  */
+	      bp_objfile_data->overlay_msym = &msym_not_found;
+	      continue;
+	    }
+	  bp_objfile_data->overlay_msym = m;
+	}
 
-      b = create_internal_breakpoint (get_objfile_arch (objfile),
-				      SYMBOL_VALUE_ADDRESS (m),
+      addr = SYMBOL_VALUE_ADDRESS (bp_objfile_data->overlay_msym);
+      b = create_internal_breakpoint (get_objfile_arch (objfile), addr,
                                       bp_overlay_event);
       b->addr_string = xstrdup (func_name);
 
@@ -2267,31 +2336,42 @@ create_longjmp_master_breakpoint (void)
 
     ALL_OBJFILES (objfile)
     {
-      const char *const longjmp_names[]
-	= { "longjmp", "_longjmp", "siglongjmp", "_siglongjmp" };
-      const int num_longjmp_names
-	= sizeof (longjmp_names) / sizeof (longjmp_names[0]);
       int i;
       struct gdbarch *gdbarch;
+      struct breakpoint_objfile_data *bp_objfile_data;
 
       gdbarch = get_objfile_arch (objfile);
       if (!gdbarch_get_longjmp_target_p (gdbarch))
 	continue;
 
-      for (i = 0; i < num_longjmp_names; i++)
+      bp_objfile_data = get_breakpoint_objfile_data (objfile);
+
+      for (i = 0; i < NUM_LONGJMP_NAMES; i++)
 	{
 	  struct breakpoint *b;
-	  struct minimal_symbol *m;
 	  const char *func_name;
+	  CORE_ADDR addr;
 
-	  func_name = longjmp_names[i];
-	  m = lookup_minimal_symbol_text (func_name, objfile);
-	  if (m == NULL)
+	  if (msym_not_found_p (bp_objfile_data->longjmp_msym[i]))
 	    continue;
 
-	  b = create_internal_breakpoint (gdbarch,
-					  SYMBOL_VALUE_ADDRESS (m),
-					  bp_longjmp_master);
+	  func_name = longjmp_names[i];
+	  if (bp_objfile_data->longjmp_msym[i] == NULL)
+	    {
+	      struct minimal_symbol *m;
+
+	      m = lookup_minimal_symbol_text (func_name, objfile);
+	      if (m == NULL)
+		{
+		  /* Prevent future lookups in this objfile.  */
+		  bp_objfile_data->longjmp_msym[i] = &msym_not_found;
+		  continue;
+		}
+	      bp_objfile_data->longjmp_msym[i] = m;
+	    }
+
+	  addr = SYMBOL_VALUE_ADDRESS (bp_objfile_data->longjmp_msym[i]);
+	  b = create_internal_breakpoint (gdbarch, addr, bp_longjmp_master);
 	  b->addr_string = xstrdup (func_name);
 	  b->enable_state = bp_disabled;
 	}
@@ -2307,31 +2387,51 @@ static void
 create_std_terminate_master_breakpoint (void)
 {
   struct program_space *pspace;
-  struct objfile *objfile;
   struct cleanup *old_chain;
   const char *const func_name = "std::terminate()";
 
   old_chain = save_current_program_space ();
 
   ALL_PSPACES (pspace)
+  {
+    struct objfile *objfile;
+    CORE_ADDR addr;
+
+    set_current_program_space (pspace);
+
     ALL_OBJFILES (objfile)
     {
       struct breakpoint *b;
-      struct minimal_symbol *m;
+      struct breakpoint_objfile_data *bp_objfile_data;
 
-      set_current_program_space (pspace);
+      bp_objfile_data = get_breakpoint_objfile_data (objfile);
 
-      m = lookup_minimal_symbol (func_name, NULL, objfile);
-      if (m == NULL || (MSYMBOL_TYPE (m) != mst_text
-			&& MSYMBOL_TYPE (m) != mst_file_text))
-        continue;
+      if (msym_not_found_p (bp_objfile_data->terminate_msym))
+	continue;
+
+      if (bp_objfile_data->terminate_msym == NULL)
+	{
+	  struct minimal_symbol *m;
+
+	  m = lookup_minimal_symbol (func_name, NULL, objfile);
+	  if (m == NULL || (MSYMBOL_TYPE (m) != mst_text
+			    && MSYMBOL_TYPE (m) != mst_file_text))
+	    {
+	      /* Prevent future lookups in this objfile.  */
+	      bp_objfile_data->terminate_msym = &msym_not_found;
+	      continue;
+	    }
+	  bp_objfile_data->terminate_msym = m;
+	}
 
-      b = create_internal_breakpoint (get_objfile_arch (objfile),
-				      SYMBOL_VALUE_ADDRESS (m),
+      addr = SYMBOL_VALUE_ADDRESS (bp_objfile_data->terminate_msym);
+      b = create_internal_breakpoint (get_objfile_arch (objfile), addr,
                                       bp_std_terminate_master);
       b->addr_string = xstrdup (func_name);
       b->enable_state = bp_disabled;
     }
+  }
+
   update_global_location_list (1);
 
   do_cleanups (old_chain);
@@ -2343,24 +2443,42 @@ void
 create_exception_master_breakpoint (void)
 {
   struct objfile *objfile;
+  const char *const func_name = "_Unwind_DebugHook";
 
   ALL_OBJFILES (objfile)
     {
-      struct minimal_symbol *debug_hook;
+      struct breakpoint *b;
+      struct gdbarch *gdbarch;
+      struct breakpoint_objfile_data *bp_objfile_data;
+      CORE_ADDR addr;
+
+      bp_objfile_data = get_breakpoint_objfile_data (objfile);
+
+      if (msym_not_found_p (bp_objfile_data->exception_msym))
+	continue;
+
+      gdbarch = get_objfile_arch (objfile);
 
-      debug_hook = lookup_minimal_symbol ("_Unwind_DebugHook", NULL, objfile);
-      if (debug_hook != NULL)
+      if (bp_objfile_data->exception_msym == NULL)
 	{
-	  struct breakpoint *b;
-	  CORE_ADDR addr = SYMBOL_VALUE_ADDRESS (debug_hook);
-	  struct gdbarch *gdbarch = get_objfile_arch (objfile);
+	  struct minimal_symbol *debug_hook;
 
-	  addr = gdbarch_convert_from_func_ptr_addr (gdbarch, addr,
-						     &current_target);
-	  b = create_internal_breakpoint (gdbarch, addr, bp_exception_master);
-	  b->addr_string = xstrdup ("_Unwind_DebugHook");
-	  b->enable_state = bp_disabled;
+	  debug_hook = lookup_minimal_symbol (func_name, NULL, objfile);
+	  if (debug_hook == NULL)
+	    {
+	      bp_objfile_data->exception_msym = &msym_not_found;
+	      continue;
+	    }
+
+	  bp_objfile_data->exception_msym = debug_hook;
 	}
+
+      addr = SYMBOL_VALUE_ADDRESS (bp_objfile_data->exception_msym);
+      addr = gdbarch_convert_from_func_ptr_addr (gdbarch, addr,
+						 &current_target);
+      b = create_internal_breakpoint (gdbarch, addr, bp_exception_master);
+      b->addr_string = xstrdup (func_name);
+      b->enable_state = bp_disabled;
     }
 
   update_global_location_list (1);
@@ -12071,6 +12189,8 @@ _initialize_breakpoint (void)
   observer_attach_inferior_exit (clear_syscall_counts);
   observer_attach_memory_changed (invalidate_bp_value_on_memory_change);
 
+  breakpoint_objfile_key = register_objfile_data ();
+
   breakpoint_chain = 0;
   /* Don't bother to call set_breakpoint_count.  $bpnum isn't useful
      before a breakpoint is set.  */