* [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
@ 2016-03-03 14:16 Patrick Palka
2016-03-03 14:22 ` Markus Trippelsdorf
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Patrick Palka @ 2016-03-03 14:16 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
push_to_top_level gets called fairly frequently in template-heavy code
that performs a lot of instantiations, and we currently "leak" a lot of
GC memory when compiling such code since [push|pop]_to_top_level() do
not bother reusing or even freeing each saved_scope structure it
allocates.
This patch makes push_to_top_level() reuse the saved_scope structures it
allocates. This is similar to how begin_scope() reuses the
cp_binding_level structures it allocates.
This patch reduces the maximum memory usage of the compiler by 4.5%,
from 525MB to 500MB, when compiling the Boost::Fusion test file
libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
trunk or for GCC 7?
gcc/cp/ChangeLog:
* name-lookup.c (free_saved_scope): New free list of saved_scope
structures.
(push_to_top_level): Attempt to reuse a saved_scope struct
from free_saved_scope instead of allocating a new one each time.
(pop_from_top_level_1): Chain the now-unused saved_scope structure
onto free_saved_scope.
---
gcc/cp/name-lookup.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 89d84d7..3478b6a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -6134,6 +6134,10 @@ store_class_bindings (vec<cp_class_binding, va_gc> *names,
timevar_cond_stop (TV_NAME_LOOKUP, subtime);
}
+/* A chain of saved_scope structures awaiting reuse. */
+
+static GTY((deletable)) struct saved_scope *free_saved_scope;
+
void
push_to_top_level (void)
{
@@ -6144,7 +6148,21 @@ push_to_top_level (void)
bool need_pop;
bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
- s = ggc_cleared_alloc<saved_scope> ();
+
+ /* Reuse or create a new structure for this saved scope. */
+ if (free_saved_scope != NULL)
+ {
+ s = free_saved_scope;
+ free_saved_scope = s->prev;
+
+ vec<cxx_saved_binding, va_gc> *old_bindings = s->old_bindings;
+ memset (s, 0, sizeof (*s));
+ /* Also reuse the structure's old_bindings vector. */
+ vec_safe_truncate (old_bindings, 0);
+ s->old_bindings = old_bindings;
+ }
+ else
+ s = ggc_cleared_alloc<saved_scope> ();
b = scope_chain ? current_binding_level : 0;
@@ -6237,6 +6255,11 @@ pop_from_top_level_1 (void)
current_function_decl = s->function_decl;
cp_unevaluated_operand = s->unevaluated_operand;
c_inhibit_evaluation_warnings = s->inhibit_evaluation_warnings;
+
+ /* Make this saved_scope structure available for reuse by
+ push_to_top_level. */
+ s->prev = free_saved_scope;
+ free_saved_scope = s;
}
/* Wrapper for pop_from_top_level_1. */
--
2.8.0.rc0.11.g9bfbc33
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 14:16 [PATCH] Reuse the saved_scope structures allocated by push_to_top_level Patrick Palka
@ 2016-03-03 14:22 ` Markus Trippelsdorf
2016-03-03 14:49 ` Patrick Palka
2016-04-18 1:49 ` Patrick Palka
2016-04-21 16:37 ` Jason Merrill
2 siblings, 1 reply; 8+ messages in thread
From: Markus Trippelsdorf @ 2016-03-03 14:22 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches, jason
On 2016.03.03 at 09:16 -0500, Patrick Palka wrote:
> push_to_top_level gets called fairly frequently in template-heavy code
> that performs a lot of instantiations, and we currently "leak" a lot of
> GC memory when compiling such code since [push|pop]_to_top_level() do
> not bother reusing or even freeing each saved_scope structure it
> allocates.
>
> This patch makes push_to_top_level() reuse the saved_scope structures it
> allocates. This is similar to how begin_scope() reuses the
> cp_binding_level structures it allocates.
>
> This patch reduces the maximum memory usage of the compiler by 4.5%,
> from 525MB to 500MB, when compiling the Boost::Fusion test file
> libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
>
> Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
> trunk or for GCC 7?
Great. push_to_top_level also shows up very high in profiles when
building Chromium for example.
There is an old bug for this issue:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64500
--
Markus
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 14:22 ` Markus Trippelsdorf
@ 2016-03-03 14:49 ` Patrick Palka
2016-03-03 15:22 ` Manuel López-Ibáñez
0 siblings, 1 reply; 8+ messages in thread
From: Patrick Palka @ 2016-03-03 14:49 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: GCC Patches, Jason Merrill
On Thu, Mar 3, 2016 at 9:22 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2016.03.03 at 09:16 -0500, Patrick Palka wrote:
>> push_to_top_level gets called fairly frequently in template-heavy code
>> that performs a lot of instantiations, and we currently "leak" a lot of
>> GC memory when compiling such code since [push|pop]_to_top_level() do
>> not bother reusing or even freeing each saved_scope structure it
>> allocates.
>>
>> This patch makes push_to_top_level() reuse the saved_scope structures it
>> allocates. This is similar to how begin_scope() reuses the
>> cp_binding_level structures it allocates.
>>
>> This patch reduces the maximum memory usage of the compiler by 4.5%,
>> from 525MB to 500MB, when compiling the Boost::Fusion test file
>> libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
>>
>> Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
>> trunk or for GCC 7?
>
> Great. push_to_top_level also shows up very high in profiles when
> building Chromium for example.
>
> There is an old bug for this issue:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64500
>
> --
> Markus
I forgot what exactly I was benchmarking but I also saw
push_to_top_level high on the list which is what made me interested in
this function in the first place.
I think the slowness of this function is mostly due to the pointer
chasing performed in the function store_bindings, where we iterate
over all the names in each non-global scope to figure out whether to
preserve them. It would probably improve performance if
cp_binding_level::names were a vector of trees instead of a linked
list of trees.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 14:49 ` Patrick Palka
@ 2016-03-03 15:22 ` Manuel López-Ibáñez
2016-03-03 15:39 ` Patrick Palka
0 siblings, 1 reply; 8+ messages in thread
From: Manuel López-Ibáñez @ 2016-03-03 15:22 UTC (permalink / raw)
To: Patrick Palka, Markus Trippelsdorf; +Cc: GCC Patches, Jason Merrill
On 03/03/16 14:49, Patrick Palka wrote:
> I think the slowness of this function is mostly due to the pointer
> chasing performed in the function store_bindings, where we iterate
> over all the names in each non-global scope to figure out whether to
> preserve them. It would probably improve performance if
> cp_binding_level::names were a vector of trees instead of a linked
> list of trees.
It would be an overall improvement if it was neither a TREE_LIST, nor a
TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees
Those kinds of cleanups are always welcome even if they do not improve
performance noticeably at first glance. The speed-up will show up once
TREE_LIST is removed completely.
Cheers,
Manuel.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 15:22 ` Manuel López-Ibáñez
@ 2016-03-03 15:39 ` Patrick Palka
2016-03-03 16:01 ` Manuel López-Ibáñez
0 siblings, 1 reply; 8+ messages in thread
From: Patrick Palka @ 2016-03-03 15:39 UTC (permalink / raw)
To: Manuel López-Ibáñez
Cc: Markus Trippelsdorf, GCC Patches, Jason Merrill
On Thu, Mar 3, 2016 at 10:22 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 03/03/16 14:49, Patrick Palka wrote:
>>
>> I think the slowness of this function is mostly due to the pointer
>> chasing performed in the function store_bindings, where we iterate
>> over all the names in each non-global scope to figure out whether to
>> preserve them. It would probably improve performance if
>> cp_binding_level::names were a vector of trees instead of a linked
>> list of trees.
>
>
> It would be an overall improvement if it was neither a TREE_LIST, nor a
> TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees
>
> Those kinds of cleanups are always welcome even if they do not improve
> performance noticeably at first glance. The speed-up will show up once
> TREE_LIST is removed completely.
Ah yeah, I meant if cp_binding_level::names were a vec<tree, va_gc>
since it would have to be resizable.
>
> Cheers,
>
> Manuel.
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 15:39 ` Patrick Palka
@ 2016-03-03 16:01 ` Manuel López-Ibáñez
0 siblings, 0 replies; 8+ messages in thread
From: Manuel López-Ibáñez @ 2016-03-03 16:01 UTC (permalink / raw)
To: Patrick Palka; +Cc: Markus Trippelsdorf, GCC Patches, Jason Merrill
On 3 March 2016 at 15:39, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Mar 3, 2016 at 10:22 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> It would be an overall improvement if it was neither a TREE_LIST, nor a
>> TREE_VECTOR: https://gcc.gnu.org/wiki/Speedup_areas#Trees
>>
>> Those kinds of cleanups are always welcome even if they do not improve
>> performance noticeably at first glance. The speed-up will show up once
>> TREE_LIST is removed completely.
>
> Ah yeah, I meant if cp_binding_level::names were a vec<tree, va_gc>
> since it would have to be resizable.
Sure, what I meant is that such a change is an improvement even if you
cannot measure any speed-up at all right now. Go for it!
Cheers,
Manuel.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 14:16 [PATCH] Reuse the saved_scope structures allocated by push_to_top_level Patrick Palka
2016-03-03 14:22 ` Markus Trippelsdorf
@ 2016-04-18 1:49 ` Patrick Palka
2016-04-21 16:37 ` Jason Merrill
2 siblings, 0 replies; 8+ messages in thread
From: Patrick Palka @ 2016-04-18 1:49 UTC (permalink / raw)
To: GCC Patches; +Cc: Jason Merrill, Patrick Palka
On Thu, Mar 3, 2016 at 9:16 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> push_to_top_level gets called fairly frequently in template-heavy code
> that performs a lot of instantiations, and we currently "leak" a lot of
> GC memory when compiling such code since [push|pop]_to_top_level() do
> not bother reusing or even freeing each saved_scope structure it
> allocates.
>
> This patch makes push_to_top_level() reuse the saved_scope structures it
> allocates. This is similar to how begin_scope() reuses the
> cp_binding_level structures it allocates.
>
> This patch reduces the maximum memory usage of the compiler by 4.5%,
> from 525MB to 500MB, when compiling the Boost::Fusion test file
> libs/fusion/test/compile_time/transform.cpp from the Boost 1.60 testsuite.
>
> Bootstrapped and tested on x86_64-pc-linux-gnu, OK for
> trunk or for GCC 7?
>
> gcc/cp/ChangeLog:
>
> * name-lookup.c (free_saved_scope): New free list of saved_scope
> structures.
> (push_to_top_level): Attempt to reuse a saved_scope struct
> from free_saved_scope instead of allocating a new one each time.
> (pop_from_top_level_1): Chain the now-unused saved_scope structure
> onto free_saved_scope.
> ---
> gcc/cp/name-lookup.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 89d84d7..3478b6a 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -6134,6 +6134,10 @@ store_class_bindings (vec<cp_class_binding, va_gc> *names,
> timevar_cond_stop (TV_NAME_LOOKUP, subtime);
> }
>
> +/* A chain of saved_scope structures awaiting reuse. */
> +
> +static GTY((deletable)) struct saved_scope *free_saved_scope;
> +
> void
> push_to_top_level (void)
> {
> @@ -6144,7 +6148,21 @@ push_to_top_level (void)
> bool need_pop;
>
> bool subtime = timevar_cond_start (TV_NAME_LOOKUP);
> - s = ggc_cleared_alloc<saved_scope> ();
> +
> + /* Reuse or create a new structure for this saved scope. */
> + if (free_saved_scope != NULL)
> + {
> + s = free_saved_scope;
> + free_saved_scope = s->prev;
> +
> + vec<cxx_saved_binding, va_gc> *old_bindings = s->old_bindings;
> + memset (s, 0, sizeof (*s));
> + /* Also reuse the structure's old_bindings vector. */
> + vec_safe_truncate (old_bindings, 0);
> + s->old_bindings = old_bindings;
> + }
> + else
> + s = ggc_cleared_alloc<saved_scope> ();
>
> b = scope_chain ? current_binding_level : 0;
>
> @@ -6237,6 +6255,11 @@ pop_from_top_level_1 (void)
> current_function_decl = s->function_decl;
> cp_unevaluated_operand = s->unevaluated_operand;
> c_inhibit_evaluation_warnings = s->inhibit_evaluation_warnings;
> +
> + /* Make this saved_scope structure available for reuse by
> + push_to_top_level. */
> + s->prev = free_saved_scope;
> + free_saved_scope = s;
> }
>
> /* Wrapper for pop_from_top_level_1. */
> --
> 2.8.0.rc0.11.g9bfbc33
>
Ping.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Reuse the saved_scope structures allocated by push_to_top_level
2016-03-03 14:16 [PATCH] Reuse the saved_scope structures allocated by push_to_top_level Patrick Palka
2016-03-03 14:22 ` Markus Trippelsdorf
2016-04-18 1:49 ` Patrick Palka
@ 2016-04-21 16:37 ` Jason Merrill
2 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2016-04-21 16:37 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-04-21 16:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-03 14:16 [PATCH] Reuse the saved_scope structures allocated by push_to_top_level Patrick Palka
2016-03-03 14:22 ` Markus Trippelsdorf
2016-03-03 14:49 ` Patrick Palka
2016-03-03 15:22 ` Manuel López-Ibáñez
2016-03-03 15:39 ` Patrick Palka
2016-03-03 16:01 ` Manuel López-Ibáñez
2016-04-18 1:49 ` Patrick Palka
2016-04-21 16:37 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).