[RFA,26/42] Remove free_pendings
Commit Message
buildsym.c currently keeps a free list of "struct pending"s. However,
this didn't seem necessary to me, and so this patch removes the free
list.
gdb/ChangeLog
2018-05-22 Tom Tromey <tom@tromey.com>
* buildsym.c (free_pendings): Remove.
(add_symbol_to_list, scoped_free_pendings)
(finish_block_internal, buildsym_init): Update.
---
gdb/ChangeLog | 6 ++++++
gdb/buildsym.c | 28 +++-------------------------
2 files changed, 9 insertions(+), 25 deletions(-)
Comments
On 2018-05-23 12:58 AM, Tom Tromey wrote:
> buildsym.c currently keeps a free list of "struct pending"s. However,
> this didn't seem necessary to me, and so this patch removes the free
> list.
LGTM.
It looks like this was to avoid some allocations/deallocations, for performance?
Since it's hard to tell if something is worth it performance-wise without data,
I made some quick measurements. I compiled GDB with optimizations, then loaded
my debug build of GDB in it like this:
$ for i in 1 2 3; do /usr/bin/time -a -o after ./gdb /home/simark/build/binutils-gdb/gdb/gdb -readnow -batch; done
This is before:
58.66user 5.38system 1:04.47elapsed 99%CPU (0avgtext+0avgdata 2841916maxresident)k
2520inputs+0outputs (1major+679737minor)pagefaults 0swaps
59.24user 5.52system 1:05.07elapsed 99%CPU (0avgtext+0avgdata 2841572maxresident)k
1616inputs+0outputs (4major+679723minor)pagefaults 0swaps
61.50user 5.38system 1:07.36elapsed 99%CPU (0avgtext+0avgdata 2841764maxresident)k
0inputs+0outputs (0major+679732minor)pagefaults 0swaps
This is after:
53.14user 4.91system 0:59.02elapsed 98%CPU (0avgtext+0avgdata 2841824maxresident)k
0inputs+0outputs (0major+679713minor)pagefaults 0swaps
52.70user 4.97system 0:58.49elapsed 98%CPU (0avgtext+0avgdata 2842052maxresident)k
0inputs+0outputs (0major+679747minor)pagefaults 0swaps
54.42user 5.30system 1:01.11elapsed 97%CPU (0avgtext+0avgdata 2841764maxresident)k
0inputs+0outputs (0major+679760minor)pagefaults 0swaps
So, unless I mixed up "before" and "after", it looks like this patch actually
improves the performance of GDB!
Simon
On 2018-07-09 10:55 PM, Simon Marchi wrote:
> On 2018-05-23 12:58 AM, Tom Tromey wrote:
>> buildsym.c currently keeps a free list of "struct pending"s. However,
>> this didn't seem necessary to me, and so this patch removes the free
>> list.
>
> LGTM.
>
> It looks like this was to avoid some allocations/deallocations, for performance?
> Since it's hard to tell if something is worth it performance-wise without data,
> I made some quick measurements. I compiled GDB with optimizations, then loaded
> my debug build of GDB in it like this:
>
> $ for i in 1 2 3; do /usr/bin/time -a -o after ./gdb /home/simark/build/binutils-gdb/gdb/gdb -readnow -batch; done
>
> This is before:
>
> 58.66user 5.38system 1:04.47elapsed 99%CPU (0avgtext+0avgdata 2841916maxresident)k
> 2520inputs+0outputs (1major+679737minor)pagefaults 0swaps
> 59.24user 5.52system 1:05.07elapsed 99%CPU (0avgtext+0avgdata 2841572maxresident)k
> 1616inputs+0outputs (4major+679723minor)pagefaults 0swaps
> 61.50user 5.38system 1:07.36elapsed 99%CPU (0avgtext+0avgdata 2841764maxresident)k
> 0inputs+0outputs (0major+679732minor)pagefaults 0swaps
>
> This is after:
>
> 53.14user 4.91system 0:59.02elapsed 98%CPU (0avgtext+0avgdata 2841824maxresident)k
> 0inputs+0outputs (0major+679713minor)pagefaults 0swaps
> 52.70user 4.97system 0:58.49elapsed 98%CPU (0avgtext+0avgdata 2842052maxresident)k
> 0inputs+0outputs (0major+679747minor)pagefaults 0swaps
> 54.42user 5.30system 1:01.11elapsed 97%CPU (0avgtext+0avgdata 2841764maxresident)k
> 0inputs+0outputs (0major+679760minor)pagefaults 0swaps
>
> So, unless I mixed up "before" and "after", it looks like this patch actually
> improves the performance of GDB!
>
> Simon
Ok, now that I re-run it I don't see a difference anymore... not sure what happened.
Still, the patch LGTM :)
Simon
@@ -258,10 +258,6 @@ struct buildsym_compunit
static struct buildsym_compunit *buildsym_compunit;
-/* List of free `struct pending' structures for reuse. */
-
-static struct pending *free_pendings;
-
/* List of blocks already made (lexical contexts already closed).
This is used at the end to make the blockvector. */
@@ -303,16 +299,7 @@ add_symbol_to_list (struct symbol *symbol, struct pending **listhead)
don't have a link with room in it, add a new link. */
if (*listhead == NULL || (*listhead)->nsyms == PENDINGSIZE)
{
- if (free_pendings)
- {
- link = free_pendings;
- free_pendings = link->next;
- }
- else
- {
- link = XNEW (struct pending);
- }
-
+ link = XNEW (struct pending);
link->next = *listhead;
*listhead = link;
link->nsyms = 0;
@@ -360,13 +347,6 @@ scoped_free_pendings::~scoped_free_pendings ()
{
struct pending *next, *next1;
- for (next = free_pendings; next; next = next1)
- {
- next1 = next->next;
- xfree ((void *) next);
- }
- free_pendings = NULL;
-
for (next = file_symbols; next != NULL; next = next1)
{
next1 = next->next;
@@ -488,13 +468,12 @@ finish_block_internal (struct symbol *symbol,
if (static_link != NULL)
objfile_register_static_link (objfile, block, static_link);
- /* Now "free" the links of the list, and empty the list. */
+ /* Now free the links of the list, and empty the list. */
for (next = *listhead; next; next = next1)
{
next1 = next->next;
- next->next = free_pendings;
- free_pendings = next;
+ xfree (next);
}
*listhead = NULL;
@@ -1767,7 +1746,6 @@ buildsym_init ()
{
/* Ensure the scoped_free_pendings destructor was called after
the last time. */
- gdb_assert (free_pendings == NULL);
gdb_assert (file_symbols == NULL);
gdb_assert (global_symbols == NULL);
gdb_assert (buildsym_compunit == NULL);