[PATCH 5/5][libbacktrace] Reduce memory usage in build_address_map

[PATCH 5/5][libbacktrace] Reduce memory usage in build_address_map

[Tom de Vries]

Hi,

In build_address_map we allocate a unit, and then look for addresses in the
unit, which we store in the addrs vector, with the elements pointing to the
unit.  However, if we cannot find addresses in the unit, the allocated unit is
not used.

Fix this by detecting if the allocated unit has been used, and reusing it
otherwise.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[libbacktrace] Reduce memory usage in build_address_map

2018-11-28  Tom de Vries  <tdevries@suse.de>

	* dwarf.c (build_address_map): Reuse unused units.

---
 libbacktrace/dwarf.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index f843fab7529..ff97a20808c 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -1436,9 +1436,11 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
   size_t units_count;
   size_t i;
   struct unit **pu;
+  size_t prev_addrs_count;
 
   memset (&addrs->vec, 0, sizeof addrs->vec);
   addrs->count = 0;
+  prev_addrs_count = 0;
 
   /* Read through the .debug_info section.  FIXME: Should we use the
      .debug_aranges section?  gdb and addr2line don't use it, but I'm
@@ -1534,6 +1536,17 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
 
       if (unit_buf.reported_underflow)
 	goto fail;
+
+      if (addrs->count == prev_addrs_count)
+	{
+	  --units_count;
+	  units.size -= sizeof (u);
+	  units.alc += sizeof (u);
+	  free_abbrevs (state, &u->abbrevs, error_callback, data);
+	  backtrace_free (state, u, sizeof *u, error_callback, data);
+	}
+      else
+	prev_addrs_count = addrs->count;
     }
   if (info.reported_underflow)
     goto fail;

Re: [PATCH 5/5][libbacktrace] Reduce memory usage in build_address_map

[Ian Lance Taylor via gcc-patches]

On Wed, Nov 28, 2018 at 3:18 PM Tom de Vries <tdevries@suse.de> wrote:
>
> In build_address_map we allocate a unit, and then look for addresses in the
> unit, which we store in the addrs vector, with the elements pointing to the
> unit.  However, if we cannot find addresses in the unit, the allocated unit is
> not used.
>
> Fix this by detecting if the allocated unit has been used, and reusing it
> otherwise.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> [libbacktrace] Reduce memory usage in build_address_map
>
> 2018-11-28  Tom de Vries  <tdevries@suse.de>
>
>         * dwarf.c (build_address_map): Reuse unused units.
>
> ---
>  libbacktrace/dwarf.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
> index f843fab7529..ff97a20808c 100644
> --- a/libbacktrace/dwarf.c
> +++ b/libbacktrace/dwarf.c
> @@ -1436,9 +1436,11 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
>    size_t units_count;
>    size_t i;
>    struct unit **pu;
> +  size_t prev_addrs_count;
>
>    memset (&addrs->vec, 0, sizeof addrs->vec);
>    addrs->count = 0;
> +  prev_addrs_count = 0;
>
>    /* Read through the .debug_info section.  FIXME: Should we use the
>       .debug_aranges section?  gdb and addr2line don't use it, but I'm
> @@ -1534,6 +1536,17 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address,
>
>        if (unit_buf.reported_underflow)
>         goto fail;
> +
> +      if (addrs->count == prev_addrs_count)
> +       {
> +         --units_count;
> +         units.size -= sizeof (u);
> +         units.alc += sizeof (u);
> +         free_abbrevs (state, &u->abbrevs, error_callback, data);
> +         backtrace_free (state, u, sizeof *u, error_callback, data);
> +       }
> +      else
> +       prev_addrs_count = addrs->count;
>      }
>    if (info.reported_underflow)
>      goto fail;

Please flip the code to make the simple case first.

if (addrs->count > prev_addrs_count)
  prev_addrs_count = addrs->count;
else
  {
    /* Unit was not used; remove it from the vector.  */
    --units_count;
    ...
  }

OK with that change.

Thanks.

Ian