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 <tdevr...@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  <tdevr...@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