[RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Pierre Muller]

  Each time, you examine the convenience variable
$_tlb on Windows executable
the function windows_get_tlb_type is called
and creates a new type.

  I copied this behavior from the internal variable _siginfo,
but I think that it is a big waste of time and memory to
regenerate it each time.
  Nevertheless, with multi-target GDB, you can possibly create
a type of windows-32 bit another for windows 64-bit
or even one for windows-CE.
  Thus I remember the last gdbarch passed
and return the remember type only if gdbarch did not change,
which is most of the time.

  I do not know if those kind of patches
are regarded as useful.

Comments welcome.


Pierre Muller

2010-04-19  Pierre Muller  <muller@ics.u-strasbg.fr>

	* windows-tdep.c (windows_get_tlb_type): Remember
	last GDBARCH and created type for re-use.

Index: windows-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-tdep.c,v
retrieving revision 1.6
diff -u -p -r1.6 windows-tdep.c
--- windows-tdep.c	16 Apr 2010 07:49:35 -0000	1.6
+++ windows-tdep.c	19 Apr 2010 08:45:27 -0000
@@ -97,12 +97,18 @@ static int maint_display_all_tib = 0;
 static struct type *
 windows_get_tlb_type (struct gdbarch *gdbarch)
 {
+  static struct gdbarch *last_gdbarch = NULL;
+  static struct type *last_tlb_type = NULL;
   struct type *dword_ptr_type, *dword32_type, *void_ptr_type;
   struct type *peb_ldr_type, *peb_ldr_ptr_type;
   struct type *peb_type, *peb_ptr_type, *list_type, *list_ptr_type;
   struct type *module_list_ptr_type;
   struct type *tib_type, *seh_type, *tib_ptr_type, *seh_ptr_type;
 
+  /* Do not rebuild type if same gdbarch as last time.  */
+  if (last_tlb_type && last_gdbarch == gdbarch)
+    return last_tlb_type;
+  
   dword_ptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit (gdbarch),
 				 1, "DWORD_PTR");
   dword32_type = arch_integer_type (gdbarch, 32,
@@ -210,6 +216,9 @@ windows_get_tlb_type (struct gdbarch *gd
 			    TYPE_LENGTH (void_ptr_type), NULL);
   TYPE_TARGET_TYPE (tib_ptr_type) = tib_type;
 
+  last_tlb_type = tib_ptr_type;
+  last_gdbarch = gdbarch;
+
   return tib_ptr_type;
 }
 

[PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Pierre Muller]

Nobody reacted to this:
http://sourceware.org/ml/gdb-patches/2010-04/msg00552.html

It helps avoid some unnecessary recomputation.

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : Monday, April 19, 2010 11:00 AM
> À : gdb-patches@sourceware.org
> Objet : [RFC/Windows] Remember last tlb type for re-use in
> windows_get_tlb_type
> 
>   Each time, you examine the convenience variable
> $_tlb on Windows executable
> the function windows_get_tlb_type is called
> and creates a new type.
> 
>   I copied this behavior from the internal variable _siginfo,
> but I think that it is a big waste of time and memory to
> regenerate it each time.
>   Nevertheless, with multi-target GDB, you can possibly create
> a type of windows-32 bit another for windows 64-bit
> or even one for windows-CE.
>   Thus I remember the last gdbarch passed
> and return the remember type only if gdbarch did not change,
> which is most of the time.
> 
>   I do not know if those kind of patches
> are regarded as useful.
> 
> Comments welcome.
> 
> 
> Pierre Muller
> 
> 2010-04-19  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* windows-tdep.c (windows_get_tlb_type): Remember
> 	last GDBARCH and created type for re-use.
> 
> Index: windows-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/windows-tdep.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 windows-tdep.c
> --- windows-tdep.c	16 Apr 2010 07:49:35 -0000	1.6
> +++ windows-tdep.c	19 Apr 2010 08:45:27 -0000
> @@ -97,12 +97,18 @@ static int maint_display_all_tib = 0;
>  static struct type *
>  windows_get_tlb_type (struct gdbarch *gdbarch)
>  {
> +  static struct gdbarch *last_gdbarch = NULL;
> +  static struct type *last_tlb_type = NULL;
>    struct type *dword_ptr_type, *dword32_type, *void_ptr_type;
>    struct type *peb_ldr_type, *peb_ldr_ptr_type;
>    struct type *peb_type, *peb_ptr_type, *list_type, *list_ptr_type;
>    struct type *module_list_ptr_type;
>    struct type *tib_type, *seh_type, *tib_ptr_type, *seh_ptr_type;
> 
> +  /* Do not rebuild type if same gdbarch as last time.  */
> +  if (last_tlb_type && last_gdbarch == gdbarch)
> +    return last_tlb_type;
> +
>    dword_ptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit
> (gdbarch),
>  				 1, "DWORD_PTR");
>    dword32_type = arch_integer_type (gdbarch, 32,
> @@ -210,6 +216,9 @@ windows_get_tlb_type (struct gdbarch *gd
>  			    TYPE_LENGTH (void_ptr_type), NULL);
>    TYPE_TARGET_TYPE (tib_ptr_type) = tib_type;
> 
> +  last_tlb_type = tib_ptr_type;
> +  last_gdbarch = gdbarch;
> +
>    return tib_ptr_type;
>  }
> 

Re: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Christopher Faylor]

On Fri, Apr 30, 2010 at 12:24:26AM +0200, Pierre Muller wrote:
>Nobody reacted to this:
>http://sourceware.org/ml/gdb-patches/2010-04/msg00552.html
>
>It helps avoid some unnecessary recomputation.

Looks ok but is this computation really so onerous that we need to add
the extra code?

cgf
>Pierre
>
>> -----Message d'origine-----
>> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] De la part de Pierre Muller
>> Envoy??: Monday, April 19, 2010 11:00 AM
>> ??: gdb-patches@sourceware.org
>> Objet?: [RFC/Windows] Remember last tlb type for re-use in
>> windows_get_tlb_type
>> 
>>   Each time, you examine the convenience variable
>> $_tlb on Windows executable
>> the function windows_get_tlb_type is called
>> and creates a new type.
>> 
>>   I copied this behavior from the internal variable _siginfo,
>> but I think that it is a big waste of time and memory to
>> regenerate it each time.
>>   Nevertheless, with multi-target GDB, you can possibly create
>> a type of windows-32 bit another for windows 64-bit
>> or even one for windows-CE.
>>   Thus I remember the last gdbarch passed
>> and return the remember type only if gdbarch did not change,
>> which is most of the time.
>> 
>>   I do not know if those kind of patches
>> are regarded as useful.
>> 
>> Comments welcome.
>> 
>> 
>> Pierre Muller
>> 
>> 2010-04-19  Pierre Muller  <muller@ics.u-strasbg.fr>
>> 
>> 	* windows-tdep.c (windows_get_tlb_type): Remember
>> 	last GDBARCH and created type for re-use.
>> 
>> Index: windows-tdep.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/windows-tdep.c,v
>> retrieving revision 1.6
>> diff -u -p -r1.6 windows-tdep.c
>> --- windows-tdep.c	16 Apr 2010 07:49:35 -0000	1.6
>> +++ windows-tdep.c	19 Apr 2010 08:45:27 -0000
>> @@ -97,12 +97,18 @@ static int maint_display_all_tib = 0;
>>  static struct type *
>>  windows_get_tlb_type (struct gdbarch *gdbarch)
>>  {
>> +  static struct gdbarch *last_gdbarch = NULL;
>> +  static struct type *last_tlb_type = NULL;
>>    struct type *dword_ptr_type, *dword32_type, *void_ptr_type;
>>    struct type *peb_ldr_type, *peb_ldr_ptr_type;
>>    struct type *peb_type, *peb_ptr_type, *list_type, *list_ptr_type;
>>    struct type *module_list_ptr_type;
>>    struct type *tib_type, *seh_type, *tib_ptr_type, *seh_ptr_type;
>> 
>> +  /* Do not rebuild type if same gdbarch as last time.  */
>> +  if (last_tlb_type && last_gdbarch == gdbarch)
>> +    return last_tlb_type;
>> +
>>    dword_ptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit
>> (gdbarch),
>>  				 1, "DWORD_PTR");
>>    dword32_type = arch_integer_type (gdbarch, 32,
>> @@ -210,6 +216,9 @@ windows_get_tlb_type (struct gdbarch *gd
>>  			    TYPE_LENGTH (void_ptr_type), NULL);
>>    TYPE_TARGET_TYPE (tib_ptr_type) = tib_type;
>> 
>> +  last_tlb_type = tib_ptr_type;
>> +  last_gdbarch = gdbarch;
>> +
>>    return tib_ptr_type;
>>  }
>> 
>
>
>

RE: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Christopher Faylor
> Envoyé : Friday, April 30, 2010 4:54 PM
> À : gdb-patches@sourceware.org; Pierre Muller
> Objet : Re: [PING] [RFC/Windows] Remember last tlb type for re-use in
> windows_get_tlb_type
> 
> On Fri, Apr 30, 2010 at 12:24:26AM +0200, Pierre Muller wrote:
> >Nobody reacted to this:
> >http://sourceware.org/ml/gdb-patches/2010-04/msg00552.html
> >
> >It helps avoid some unnecessary recomputation.
> 
> Looks ok but is this computation really so onerous that we need to add
> the extra code?

  Each time you dereference $_tlb
you recreate the same bunch of internal types
leaving the other types in the wild...
  This looses computation time and memory.
  Imagine that you use a long script with
a breakpoint that has commands associated
that print $_tlb.current_seh to check if the current structured exception
handler has changed....

  Honestly, I don't know if it can become a big problem,
but I thought it would be better to avoid it.
  I first was thinking about removing old values,
but I didn't find a clean way to remove the type and all its
dependents...

Pierre Muller

Re: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   I first was thinking about removing old values,
Pierre> but I didn't find a clean way to remove the type and all its
Pierre> dependents...

I was wondering if there is some way to attach them to the gdbarch, so
they can be looked up instead of recreated.

However, I think your current patch is probably just as good in practice.

Tom

Re: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Christopher Faylor]

On Fri, Apr 30, 2010 at 09:22:01AM -0600, Tom Tromey wrote:
>>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:
>
>Pierre>   I first was thinking about removing old values,
>Pierre> but I didn't find a clean way to remove the type and all its
>Pierre> dependents...
>
>I was wondering if there is some way to attach them to the gdbarch, so
>they can be looked up instead of recreated.
>
>However, I think your current patch is probably just as good in practice.

Attaching them to gdbarch would make them permanent for the life of the
debugging session right?  That seems better but I agree that, in practice,
this is probably just as good.

So, feel free to check in.

cgf

RE: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Christopher Faylor
> Envoyé : Friday, April 30, 2010 5:35 PM
> À : gdb-patches@sourceware.org; Tom Tromey; Pierre Muller
> Objet : Re: [PING] [RFC/Windows] Remember last tlb type for re-use in
> windows_get_tlb_type
> 
> On Fri, Apr 30, 2010 at 09:22:01AM -0600, Tom Tromey wrote:
> >>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
> writes:
> >
> >Pierre>   I first was thinking about removing old values,
> >Pierre> but I didn't find a clean way to remove the type and all its
> >Pierre> dependents...
> >
> >I was wondering if there is some way to attach them to the gdbarch, so
> >they can be looked up instead of recreated.
> >
> >However, I think your current patch is probably just as good in
> practice.
> 
> Attaching them to gdbarch would make them permanent for the life of the
> debugging session right?  That seems better but I agree that, in
> practice,
> this is probably just as good.
> 
> So, feel free to check in.

  Thanks for the approval,
patch committed.

Pierre

Re: [PING] [RFC/Windows] Remember last tlb type for re-use in windows_get_tlb_type

[Tom Tromey]

>>>>> "cgf" == Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org> writes:

cgf> Attaching them to gdbarch would make them permanent for the life of the
cgf> debugging session right?  That seems better but I agree that, in practice,
cgf> this is probably just as good.

Yeah, but it would fix a leak that is latent even with Pierre's latest
patch: if you use the TLB, switch arches, use it again, then switch
back, and use it a third time, then that third time will recreate the
types.  If the types were attached to the gdbarch, then the third use
would reuse the types created the first time through.

My guess is that this sort of scenario is rare.

Tom