diff mbox series

Avoid an allocation in attr_to_dynamic_prop

Message ID 20240204191559.4007457-1-tom@tromey.com
State New
Headers
Series Avoid an allocation in attr_to_dynamic_prop |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Feb. 4, 2024, 7:15 p.m. UTC 
  I noticed that attr_to_dynamic_prop allocates a dwarf_block, when no
allocation is required.  This patch stack-allocates the object
instead.
---
 gdb/dwarf2/read.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Tom de Vries Feb. 5, 2024, 4:36 a.m. UTC | #1 
On 2/4/24 20:15, Tom Tromey wrote:
> -      struct dwarf_block *block;
> +      struct dwarf_block block;
   ...
> -	block = attr->as_block ();
> +	block = *attr->as_block ();

I noticed that this replaces a pointer copy with a struct copy.  Perhaps 
that's not very relevant because the the struct is still small, and the 
compiler might optimize the copy away.

But I suppose you could consider doing the micro-optimization of:
...
-      struct dwarf_block *block;
+      struct dwarf_block local_block;
+      struct dwarf_block* block = &local_block;
...
and keep the pointer copy.

Anyway, regardless, LGTM.

Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom
Tom Tromey Feb. 5, 2024, 7:12 p.m. UTC | #2 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> But I suppose you could consider doing the micro-optimization of:
Tom> ...
Tom> -      struct dwarf_block *block;
Tom> +      struct dwarf_block local_block;
Tom> +      struct dwarf_block* block = &local_block;
Tom> ...
Tom> and keep the pointer copy.

IME these micro-optimizations don't really affect the outcome much.  I
tried a bunch like this back in the psymtab days without real effect.

Probably the baton should have a dwarf_block object and then there'd
just be a single copy here.

Tom> Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks

Tom
diff mbox series

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e873d9cc440..34bbb6e804d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -15385,24 +15385,23 @@  attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
       baton->locexpr.per_cu = cu->per_cu;
       baton->locexpr.per_objfile = per_objfile;
 
-      struct dwarf_block *block;
+      struct dwarf_block block;
       if (attr->form == DW_FORM_data16)
 	{
 	  size_t data_size = 16;
-	  block = XOBNEW (obstack, struct dwarf_block);
-	  block->size = (data_size
-			 + 2 /* Extra bytes for DW_OP and arg.  */);
-	  gdb_byte *data = XOBNEWVEC (obstack, gdb_byte, block->size);
+	  block.size = (data_size
+			+ 2 /* Extra bytes for DW_OP and arg.  */);
+	  gdb_byte *data = XOBNEWVEC (obstack, gdb_byte, block.size);
 	  data[0] = DW_OP_implicit_value;
 	  data[1] = data_size;
 	  memcpy (&data[2], attr->as_block ()->data, data_size);
-	  block->data = data;
+	  block.data = data;
 	}
       else
-	block = attr->as_block ();
+	block = *attr->as_block ();
 
-      baton->locexpr.size = block->size;
-      baton->locexpr.data = block->data;
+      baton->locexpr.size = block.size;
+      baton->locexpr.data = block.data;
       switch (attr->name)
 	{
 	case DW_AT_string_length: