| Message ID | 20181128231845.GA4067@delia |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5,libbacktrace] Factor out backtrace_vector_free | expand |
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
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;