[PATCH] remove registers_changed from switch_to_thread

[PATCH] remove registers_changed from switch_to_thread

[Yao Qi]

Hi,
When reading the code, I find registers_changed is called from
switch_to_thread, which confuses me a little bit.  We have
regcache, and each thread has its own regcache.  Why do we have
to invalidate all regcaches during thread switch?

The line of calling registers_changed in switch_to_thread is quite
old, it was written in gdb 4.10 (1993).  At that time, we didn't
have regcache, and register state is a global state.  There is also
a global variable 'registers_pid' associated with the global
register state.  When thread switches, 'registers_pid' should be
updated.  That is the reason why registers_changed should
be called in switch_to_thread.  So we don't have to do it now.
This patch is to remove this line.  Regression tested on x86_64
native and gdbserver.  OK to apply?

Note that this patch hightly reduced the allocation and
deallocation of regcache, especially in multi-threaded program.
Here are some statistics data I collected when running testsuite.

 (gdb) PASS: gdb.multi/base.exp: check remove-inferiors
 maintenance info statistics^M
 Name       Allocate Free     ^M
 frame info 19       8        ^M
-regcache   36       35       ^M
+regcache   8        7

 (gdb) PASS: gdb.threads/create-fail.exp: iteration 3: run till end
 maintenance info statistics^M
 Name       Allocate Free     ^M
 frame info 156      78       ^M
-regcache   269      269      ^M
+regcache   103      103      ^M

 (gdb) PASS: gdb.threads/create-fail.exp: iteration 4: run till end
 maintenance info statistics^M
 Name       Allocate Free     ^M
 frame info 156      78       ^M
-regcache   315      315      ^M
+regcache   96       96       ^M

gdb:

2012-07-26  Yao Qi  <yao@codesourcery.com>

	* thread.c (switch_to_thread): Don't call registers_changed.
---
 gdb/thread.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index ff5220d..7e8eec5 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -987,7 +987,6 @@ switch_to_thread (ptid_t ptid)
 
   inferior_ptid = ptid;
   reinit_frame_cache ();
-  registers_changed ();
 
   /* We don't check for is_stopped, because we're called at times
      while in the TARGET_RUNNING state, e.g., while handling an
-- 
1.7.7.6

Re: [PATCH] remove registers_changed from switch_to_thread

[Pedro Alves]

On 07/26/2012 03:44 PM, Yao Qi wrote:

> When reading the code, I find registers_changed is called from
> switch_to_thread, which confuses me a little bit.  We have
> regcache, and each thread has its own regcache.  Why do we have
> to invalidate all regcaches during thread switch?
> 
> The line of calling registers_changed in switch_to_thread is quite
> old, it was written in gdb 4.10 (1993).  At that time, we didn't
> have regcache, and register state is a global state.  There is also
> a global variable 'registers_pid' associated with the global
> register state.  When thread switches, 'registers_pid' should be
> updated.  That is the reason why registers_changed should
> be called in switch_to_thread.  So we don't have to do it now.
> This patch is to remove this line.  Regression tested on x86_64
> native and gdbserver.  OK to apply?

Nice!  OK.

-- 
Pedro Alves

[committed]: [PATCH] remove registers_changed from switch_to_thread

[Yao Qi]

On Thursday, July 26, 2012 03:57:12 PM Pedro Alves wrote:
> On 07/26/2012 03:44 PM, Yao Qi wrote:
> > When reading the code, I find registers_changed is called from
> > switch_to_thread, which confuses me a little bit.  We have
> > regcache, and each thread has its own regcache.  Why do we have
> > to invalidate all regcaches during thread switch?
> > 
> > The line of calling registers_changed in switch_to_thread is quite
> > old, it was written in gdb 4.10 (1993).  At that time, we didn't
> > have regcache, and register state is a global state.  There is also
> > a global variable 'registers_pid' associated with the global
> > register state.  When thread switches, 'registers_pid' should be
> > updated.  That is the reason why registers_changed should
> > be called in switch_to_thread.  So we don't have to do it now.
> > This patch is to remove this line.  Regression tested on x86_64
> > native and gdbserver.  OK to apply?
> 
> Nice!  OK.

Committed.

-- 
Yao (齐尧)