[5/8] Add per-unit obstack

Message ID 20200208152758.29385-6-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Feb. 8, 2020, 3:27 p.m. UTC
  This adds an auto_obstack to the DWARF frame comp_unit object, and
then changes the remaining code here to use the comp_unit obstack
rather than the objfile obstack.

At this point, all the storage for frame data is self-contained --
that is, it is independent of the objfile.

gdb/ChangeLog
2020-02-08  Tom Tromey  <tom@tromey.com>

	* dwarf2/frame.c (struct comp_unit) <obstack>: New member.
	(decode_frame_entry_1): Use the comp_unit obstack.

Change-Id: I8a1af090bcc2811762a38afbbea1512be7d952fb
---
 gdb/ChangeLog      | 5 +++++
 gdb/dwarf2/frame.c | 7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
  

Comments

Luis Machado Feb. 11, 2020, 10:34 a.m. UTC | #1
On 2/8/20 12:27 PM, Tom Tromey wrote:
> This adds an auto_obstack to the DWARF frame comp_unit object, and
> then changes the remaining code here to use the comp_unit obstack
> rather than the objfile obstack.
> 
> At this point, all the storage for frame data is self-contained --
> that is, it is independent of the objfile.
> 
> gdb/ChangeLog
> 2020-02-08  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/frame.c (struct comp_unit) <obstack>: New member.
> 	(decode_frame_entry_1): Use the comp_unit obstack.
> 
> Change-Id: I8a1af090bcc2811762a38afbbea1512be7d952fb
> ---
>   gdb/ChangeLog      | 5 +++++
>   gdb/dwarf2/frame.c | 7 +++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 0e74b8e7e68..7e1a744513b 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -158,6 +158,9 @@ struct comp_unit
>   
>     /* The FDE table.  */
>     dwarf2_fde_table fde_table;
> +
> +  /* Hold data used by this module.  */
> +  auto_obstack obstack;
>   };
>   
>   static struct dwarf2_fde *dwarf2_frame_find_fde (CORE_ADDR *pc,
> @@ -1771,7 +1774,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
>         if (find_cie (cie_table, cie_pointer))
>   	return end;
>   
> -      cie = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_cie);
> +      cie = XOBNEW (&unit->obstack, struct dwarf2_cie);
>         cie->initial_instructions = NULL;
>         cie->cie_pointer = cie_pointer;
>   
> @@ -1950,7 +1953,7 @@ decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
>         if (cie_pointer >= unit->dwarf_frame_size)
>   	return NULL;
>   
> -      fde = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_fde);
> +      fde = XOBNEW (&unit->obstack, struct dwarf2_fde);
>         fde->cie = find_cie (cie_table, cie_pointer);
>         if (fde->cie == NULL)
>   	{
> 

LGTM.
  
Simon Marchi Feb. 12, 2020, 3:53 a.m. UTC | #2
On 2020-02-08 10:27 a.m., Tom Tromey wrote:
> This adds an auto_obstack to the DWARF frame comp_unit object, and
> then changes the remaining code here to use the comp_unit obstack
> rather than the objfile obstack.
> 
> At this point, all the storage for frame data is self-contained --
> that is, it is independent of the objfile.

Would it be simpler to eliminate the obstack by making dwarf2_cie_table
and dwarf2_fde_table hold objects instead of pointers?  On top of that,
it should be good for performance as well, at least when doing lookups.

The only issue is that dwarf2_cie objects are referenced by address by
the FDEs, so we must make sure they don't get moved by the container.
But I believe std::unordered_map guarantees that (disabling copy and
assign on dwarf2_cie would verify it).  Or worst case, the it could
be an std::unordered_map of std::unique_ptr.

Simon
  
Tom Tromey Feb. 12, 2020, 10:41 p.m. UTC | #3
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Would it be simpler to eliminate the obstack by making dwarf2_cie_table
Simon> and dwarf2_fde_table hold objects instead of pointers?  On top of that,
Simon> it should be good for performance as well, at least when doing lookups.

Simon> The only issue is that dwarf2_cie objects are referenced by address by
Simon> the FDEs, so we must make sure they don't get moved by the container.
Simon> But I believe std::unordered_map guarantees that (disabling copy and
Simon> assign on dwarf2_cie would verify it).  Or worst case, the it could
Simon> be an std::unordered_map of std::unique_ptr.

I think I would rather defer this change, like the other one, since it's
not directly related to the purpose of the series.

Tom
  
Simon Marchi Feb. 12, 2020, 10:48 p.m. UTC | #4
On 2020-02-12 5:41 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> Would it be simpler to eliminate the obstack by making dwarf2_cie_table
> Simon> and dwarf2_fde_table hold objects instead of pointers?  On top of that,
> Simon> it should be good for performance as well, at least when doing lookups.
> 
> Simon> The only issue is that dwarf2_cie objects are referenced by address by
> Simon> the FDEs, so we must make sure they don't get moved by the container.
> Simon> But I believe std::unordered_map guarantees that (disabling copy and
> Simon> assign on dwarf2_cie would verify it).  Or worst case, the it could
> Simon> be an std::unordered_map of std::unique_ptr.
> 
> I think I would rather defer this change, like the other one, since it's
> not directly related to the purpose of the series.
> 
> Tom
> 

No problem, I suggested this in case it actually made things simpler, but I am
fine with what you have.

Simon
  
Tom Tromey Feb. 12, 2020, 10:51 p.m. UTC | #5
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> No problem, I suggested this in case it actually made things
Simon> simpler, but I am fine with what you have.

Thanks.  I'm going to push this series now.

Tom
  

Patch

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 0e74b8e7e68..7e1a744513b 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -158,6 +158,9 @@  struct comp_unit
 
   /* The FDE table.  */
   dwarf2_fde_table fde_table;
+
+  /* Hold data used by this module.  */
+  auto_obstack obstack;
 };
 
 static struct dwarf2_fde *dwarf2_frame_find_fde (CORE_ADDR *pc,
@@ -1771,7 +1774,7 @@  decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
       if (find_cie (cie_table, cie_pointer))
 	return end;
 
-      cie = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_cie);
+      cie = XOBNEW (&unit->obstack, struct dwarf2_cie);
       cie->initial_instructions = NULL;
       cie->cie_pointer = cie_pointer;
 
@@ -1950,7 +1953,7 @@  decode_frame_entry_1 (struct comp_unit *unit, const gdb_byte *start,
       if (cie_pointer >= unit->dwarf_frame_size)
 	return NULL;
 
-      fde = XOBNEW (&unit->objfile->objfile_obstack, struct dwarf2_fde);
+      fde = XOBNEW (&unit->obstack, struct dwarf2_fde);
       fde->cie = find_cie (cie_table, cie_pointer);
       if (fde->cie == NULL)
 	{