This is the mail archive of the gdb-patches@sources.redhat.com 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]

[commit] Eliminate prev_func_name global


Hello,

This patch eliminates the global prev_func_name from "infrun.c". See: http://sources.redhat.com/ml/gdb-patches/2003-04/msg00316.html for the rationale.

Rather than doing a GDB wide introduction of pc_in_sigtramp(pc), I've kept it local to "infrun.c". I found that the other PC_IN_SIGTRAMP calls looked like candidates for replacement with ``get_frame_type() == SIGTRAMP_FRAME''.

committed,
Andrew
2003-05-04  Andrew Cagney  <cagney@redhat.com>

	* gdbthread.h (save_infrun_state): Drop prev_func_name parameter.
	(load_infrun_state): Ditto.
	(struct thread_info): Drop "prev_func_name" field.
	* thread.c (load_infrun_state): Update.
	(save_infrun_state): Update.
	* infrun.c (prev_func_name): Delete variable.
	(init_wait_for_inferior): Do not clear prev_func_name.
	(stop_stepping, keep_going, context_switch): Do not swap
	prev_func_name.
	(handle_inferior_event, check_sigtramp2): Use pc_in_sigtramp
	instead of PC_IN_SIGTRAMP.

Index: gdbthread.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbthread.h,v
retrieving revision 1.8
diff -u -r1.8 gdbthread.h
--- gdbthread.h	12 Apr 2003 17:48:54 -0000	1.8
+++ gdbthread.h	5 May 2003 00:17:47 -0000
@@ -44,7 +44,6 @@
   int num;			/* Convenient handle (GDB thread id) */
   /* State from wait_for_inferior */
   CORE_ADDR prev_pc;
-  char *prev_func_name;
   struct breakpoint *step_resume_breakpoint;
   struct breakpoint *through_sigtramp_breakpoint;
   CORE_ADDR step_range_start;
@@ -117,7 +116,6 @@
 /* infrun context switch: save the debugger state for the given thread.  */
 extern void save_infrun_state (ptid_t ptid,
 			       CORE_ADDR prev_pc,
-			       char     *prev_func_name,
 			       int       trap_expected,
 			       struct breakpoint *step_resume_breakpoint,
 			       struct breakpoint *through_sigtramp_breakpoint,
@@ -137,7 +135,6 @@
    for the given thread.  */
 extern void load_infrun_state (ptid_t ptid,
 			       CORE_ADDR *prev_pc,
-			       char     **prev_func_name,
 			       int       *trap_expected,
 			       struct breakpoint **step_resume_breakpoint,
 			       struct breakpoint **through_sigtramp_breakpoint,
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.107
diff -u -r1.107 infrun.c
--- infrun.c	12 Apr 2003 17:48:54 -0000	1.107
+++ infrun.c	5 May 2003 00:17:51 -0000
@@ -786,12 +786,10 @@
     }
 }
 
-/* Record the pc and sp of the program the last time it stopped.
-   These are just used internally by wait_for_inferior, but need
-   to be preserved over calls to it and cleared when the inferior
-   is started.  */
+/* Record the pc of the program the last time it stopped.  This is
+   just used internally by wait_for_inferior, but need to be preserved
+   over calls to it and cleared when the inferior is started.  */
 static CORE_ADDR prev_pc;
-static char *prev_func_name;
 
 
 /* Start remote-debugging of a machine over a serial link.  */
@@ -829,7 +827,6 @@
 {
   /* These are meaningless until the first time through wait_for_inferior.  */
   prev_pc = 0;
-  prev_func_name = NULL;
 
 #ifdef HP_OS_BUG
   trap_expected_after_continue = 0;
@@ -1116,7 +1113,7 @@
   if (in_thread_list (inferior_ptid) && in_thread_list (ecs->ptid))
     {				/* Perform infrun state context switch: */
       /* Save infrun state for the old thread.  */
-      save_infrun_state (inferior_ptid, prev_pc, prev_func_name,
+      save_infrun_state (inferior_ptid, prev_pc,
 			 trap_expected, step_resume_breakpoint,
 			 through_sigtramp_breakpoint, step_range_start,
 			 step_range_end, &step_frame_id,
@@ -1127,7 +1124,7 @@
 			 ecs->current_line, ecs->current_symtab, step_sp);
 
       /* Load infrun state for the new thread.  */
-      load_infrun_state (ecs->ptid, &prev_pc, &prev_func_name,
+      load_infrun_state (ecs->ptid, &prev_pc,
 			 &trap_expected, &step_resume_breakpoint,
 			 &through_sigtramp_breakpoint, &step_range_start,
 			 &step_range_end, &step_frame_id,
@@ -1140,6 +1137,39 @@
   inferior_ptid = ecs->ptid;
 }
 
+/* Wrapper for PC_IN_SIGTRAMP that takes care of the need to find the
+   function's name.
+
+   In a classic example of "left hand VS right hand", "infrun.c" was
+   trying to improve GDB's performance by caching the result of calls
+   to calls to find_pc_partial_funtion, while at the same time
+   find_pc_partial_function was also trying to ramp up performance by
+   caching its most recent return value.  The below makes the the
+   function find_pc_partial_function solely responsibile for
+   performance issues (the local cache that relied on a global
+   variable - arrrggg - deleted).
+
+   Using the testsuite and gcov, it was found that dropping the local
+   "infrun.c" cache and instead relying on find_pc_partial_function
+   increased the number of calls to 12000 (from 10000), but the number
+   of times find_pc_partial_function's cache missed (this is what
+   matters) was only increased by only 4 (to 3569).  (A quick back of
+   envelope caculation suggests that the extra 2000 function calls
+   @1000 extra instructions per call make the 1 MIP VAX testsuite run
+   take two extra seconds, oops :-)
+
+   Long term, this function can be eliminated, replaced by the code:
+   get_frame_type(current_frame()) == SIGTRAMP_FRAME (for new
+   architectures this is very cheap).  */
+
+static int
+pc_in_sigtramp (CORE_ADDR pc)
+{
+  char *name;
+  find_pc_partial_function (pc, &name, NULL, NULL);
+  return PC_IN_SIGTRAMP (pc, name);
+}
+
 
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
@@ -2262,8 +2292,8 @@
   ecs->update_step_sp = 1;
 
   /* Did we just take a signal?  */
-  if (PC_IN_SIGTRAMP (stop_pc, ecs->stop_func_name)
-      && !PC_IN_SIGTRAMP (prev_pc, prev_func_name)
+  if (pc_in_sigtramp (stop_pc)
+      && !pc_in_sigtramp (prev_pc)
       && INNER_THAN (read_sp (), step_sp))
     {
       /* We've just taken a signal; go until we are back to
@@ -2373,7 +2403,7 @@
 	{
 	  /* We're doing a "next".  */
 
-	  if (PC_IN_SIGTRAMP (stop_pc, ecs->stop_func_name)
+	  if (pc_in_sigtramp (stop_pc)
 	      && frame_id_inner (step_frame_id,
 				 frame_id_build (read_sp (), 0)))
 	    /* We stepped out of a signal handler, and into its
@@ -2562,8 +2592,8 @@
 check_sigtramp2 (struct execution_control_state *ecs)
 {
   if (trap_expected
-      && PC_IN_SIGTRAMP (stop_pc, ecs->stop_func_name)
-      && !PC_IN_SIGTRAMP (prev_pc, prev_func_name)
+      && pc_in_sigtramp (stop_pc)
+      && !pc_in_sigtramp (prev_pc)
       && INNER_THAN (read_sp (), step_sp))
     {
       /* What has happened here is that we have just stepped the
@@ -2733,7 +2763,6 @@
          time, just like we did above if we didn't break out of the
          loop.  */
       prev_pc = read_pc ();
-      prev_func_name = ecs->stop_func_name;
     }
 
   /* Let callers know we don't want to wait for the inferior anymore.  */
@@ -2749,7 +2778,6 @@
 {
   /* Save the pc before execution, to compare with pc after stop.  */
   prev_pc = read_pc ();		/* Might have been DECR_AFTER_BREAK */
-  prev_func_name = ecs->stop_func_name;
 
   if (ecs->update_step_sp)
     step_sp = read_sp ();
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.30
diff -u -r1.30 thread.c
--- thread.c	12 Apr 2003 17:48:55 -0000	1.30
+++ thread.c	5 May 2003 00:17:52 -0000
@@ -292,7 +292,6 @@
 void
 load_infrun_state (ptid_t ptid,
 		   CORE_ADDR *prev_pc,
-		   char **prev_func_name,
 		   int *trap_expected,
 		   struct breakpoint **step_resume_breakpoint,
 		   struct breakpoint **through_sigtramp_breakpoint,
@@ -316,7 +315,6 @@
     return;
 
   *prev_pc = tp->prev_pc;
-  *prev_func_name = tp->prev_func_name;
   *trap_expected = tp->trap_expected;
   *step_resume_breakpoint = tp->step_resume_breakpoint;
   *through_sigtramp_breakpoint = tp->through_sigtramp_breakpoint;
@@ -340,7 +338,6 @@
 void
 save_infrun_state (ptid_t ptid,
 		   CORE_ADDR prev_pc,
-		   char *prev_func_name,
 		   int trap_expected,
 		   struct breakpoint *step_resume_breakpoint,
 		   struct breakpoint *through_sigtramp_breakpoint,
@@ -364,7 +361,6 @@
     return;
 
   tp->prev_pc = prev_pc;
-  tp->prev_func_name = prev_func_name;
   tp->trap_expected = trap_expected;
   tp->step_resume_breakpoint = step_resume_breakpoint;
   tp->through_sigtramp_breakpoint = through_sigtramp_breakpoint;

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