[5/8] Add per-unit obstack

Message ID 20200208152758.29385-6-tom@tromey.com
State New
Headers show
Series
  • Share DWARF frame information across inferiors
Related show

Commit Message

Tom Tromey Feb. 8, 2020, 3:27 p.m.
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(-)

-- 
2.17.2

Comments

Luis Machado Feb. 11, 2020, 10:34 a.m. | #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. | #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. | #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. | #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. | #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)
 	{