Re: Avoid unnecesary GGC runs during LTO
+
+ /* At this stage we know that majority of GGC memory is reachable.
+ Growing the limits prevents unnecesary invocation of GGC. */
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING, but
shouldn't this be abstracted away, thus call ggc_collect from ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust G.allocated_last_gc
and simply drop the ggc_collect above ().
I am fine with both. I basically decided to keep the explicit ggc_collect()
to
make it clear (from lto.c source code) that we are GGC safe at this point and
to have way to double check that we do not produce too much of garbage with
checking disabled. (so with -Q I will see how much it is collected at that
place).
We can embed it into ggc_grow and document that w/o checking it is equivalent
to ggc_cooect.
Anyway, this is sth for stage1 at this point.
OK,
Honza
Ping...
the patches saves 33 GGC runs during libxul.so link, that is not that bad ;)
Honza
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in their data.
*/
Index: ggc-none.c
===
--- ggc-none.c(revision 209170)
+++ ggc-none.c(working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c(revision 209170)
+++ ggc-page.c(working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits for next
collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+ G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long) G.allocated /
1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: Avoid unnecesary GGC runs during LTO
On April 17, 2014 6:03:13 PM CEST, Jan Hubicka hubi...@ucw.cz wrote:
+
+ /* At this stage we know that majority of GGC memory is
reachable.
+ Growing the limits prevents unnecesary invocation of GGC.
*/
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING,
but
shouldn't this be abstracted away, thus call ggc_collect from
ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust G.allocated_last_gc
and simply drop the ggc_collect above ().
I am fine with both. I basically decided to keep the explicit
ggc_collect() to
make it clear (from lto.c source code) that we are GGC safe at this
point and
to have way to double check that we do not produce too much of
garbage with
checking disabled. (so with -Q I will see how much it is collected at
that place).
We can embed it into ggc_grow and document that w/o checking it is
equivalent
to ggc_cooect.
Anyway, this is sth for stage1 at this point.
OK,
Honza
Ping...
the patches saves 33 GGC runs during libxul.so link, that is not that
bad ;)
What is the updated patch you propose?
Richard
Honza
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in
their data. */
Index: ggc-none.c
===
--- ggc-none.c (revision 209170)
+++ ggc-none.c (working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c (revision 209170)
+++ ggc-page.c (working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits
for next collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+ G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long)
G.allocated / 1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: Avoid unnecesary GGC runs during LTO
On April 17, 2014 6:03:13 PM CEST, Jan Hubicka hubi...@ucw.cz wrote:
+
+ /* At this stage we know that majority of GGC memory is
reachable.
+ Growing the limits prevents unnecesary invocation of GGC.
*/
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING,
but
shouldn't this be abstracted away, thus call ggc_collect from
ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust G.allocated_last_gc
and simply drop the ggc_collect above ().
I am fine with both. I basically decided to keep the explicit
ggc_collect() to
make it clear (from lto.c source code) that we are GGC safe at this
point and
to have way to double check that we do not produce too much of
garbage with
checking disabled. (so with -Q I will see how much it is collected at
that place).
We can embed it into ggc_grow and document that w/o checking it is
equivalent
to ggc_cooect.
Anyway, this is sth for stage1 at this point.
OK,
Honza
Ping...
the patches saves 33 GGC runs during libxul.so link, that is not that
bad ;)
What is the updated patch you propose?
I was trying to explain, why I kept explicit ggc_collect just after ggc_grow:
I want to make it clear that we are ggc safe at that point. I also want to see
the ggc run happening w/o checking to have -Q report how much of garbage we see
at this stage so I can keep eye on it.
I can hide ENABLE_CHECKING ggc_collect call in ggc_grow and update
documentation if your preffer.
Honza
Richard
Honza
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in
their data. */
Index: ggc-none.c
===
--- ggc-none.c (revision 209170)
+++ ggc-none.c (working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c (revision 209170)
+++ ggc-page.c (working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits
for next collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+ G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long)
G.allocated / 1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: Avoid unnecesary GGC runs during LTO
On April 17, 2014 7:18:05 PM CEST, Jan Hubicka hubi...@ucw.cz wrote:
On April 17, 2014 6:03:13 PM CEST, Jan Hubicka hubi...@ucw.cz
wrote:
+
+ /* At this stage we know that majority of GGC memory is
reachable.
+ Growing the limits prevents unnecesary invocation of
GGC.
*/
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING,
but
shouldn't this be abstracted away, thus call ggc_collect from
ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust
G.allocated_last_gc
and simply drop the ggc_collect above ().
I am fine with both. I basically decided to keep the explicit
ggc_collect() to
make it clear (from lto.c source code) that we are GGC safe at
this
point and
to have way to double check that we do not produce too much of
garbage with
checking disabled. (so with -Q I will see how much it is collected
at
that place).
We can embed it into ggc_grow and document that w/o checking it is
equivalent
to ggc_cooect.
Anyway, this is sth for stage1 at this point.
OK,
Honza
Ping...
the patches saves 33 GGC runs during libxul.so link, that is not
that
bad ;)
What is the updated patch you propose?
I was trying to explain, why I kept explicit ggc_collect just after
ggc_grow:
I want to make it clear that we are ggc safe at that point. I also want
to see
the ggc run happening w/o checking to have -Q report how much of
garbage we see
at this stage so I can keep eye on it.
I can hide ENABLE_CHECKING ggc_collect call in ggc_grow and update
documentation if your preffer.
I'd prefer that. OK with that change.
Thanks,
Richard.
Honza
Richard
Honza
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in
their data. */
Index: ggc-none.c
===
--- ggc-none.c(revision 209170)
+++ ggc-none.c(working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c(revision 209170)
+++ ggc-page.c(working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the
limits
for next collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+ G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long)
G.allocated / 1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: Avoid unnecesary GGC runs during LTO
On Fri, 11 Apr 2014, Jan Hubicka wrote:
Hi,
while looking into -ftime-report, I noticed that ggc can take up to 10% of
WPA memory
while it does almost nothing: it is run just after streaming that explicitly
frees memory that becomes unreachable. The first GGC run usually saves at
most 1% of memory and then it is never run again.
I believe this ought to also help in case we get into swap, since ltranses
will
also ggc less.
Bootstrapped/regtested x86_64-linux, OK?
Honza
* lto.c (read_cgraph_and_symbols): Grow ggc memory after streaming.
* ggc.h (ggc_grow): New function.
* ggc-none.c (ggc_grow): New function.
* ggc-page.c (ggc_grow): Likewise.
Index: ggc.h
===
--- ggc.h (revision 209170)
+++ ggc.h (working copy)
@@ -225,6 +225,9 @@ extern const char *ggc_alloc_string_stat
function is called, not during allocations. */
extern void ggc_collect (void);
+/* Assume that all GGC memory is reachable and grow the limits for next
collection. */
+extern void ggc_grow (void);
+
/* Register an additional root table. This can be useful for some
plugins. Does nothing if the passed pointer is NULL. */
extern void ggc_register_root_tab (const struct ggc_root_tab *);
Index: lto/lto.c
===
--- lto/lto.c (revision 209170)
+++ lto/lto.c (working copy)
@@ -2999,6 +3000,10 @@ read_cgraph_and_symbols (unsigned nfiles
gimple_canonical_types = NULL;
delete canonical_type_hash_cache;
canonical_type_hash_cache = NULL;
+
+ /* At this stage we know that majority of GGC memory is reachable.
+ Growing the limits prevents unnecesary invocation of GGC. */
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING, but
shouldn't this be abstracted away, thus call ggc_collect from ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust G.allocated_last_gc
and simply drop the ggc_collect above ().
Anyway, this is sth for stage1 at this point.
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in their data. */
Index: ggc-none.c
===
--- ggc-none.c(revision 209170)
+++ ggc-none.c(working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c(revision 209170)
+++ ggc-page.c(working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits for next
collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+ G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long) G.allocated /
1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: Avoid unnecesary GGC runs during LTO
On 04/11/2014 08:07 AM, Jan Hubicka wrote:
Hi,
while looking into -ftime-report, I noticed that ggc can take up to 10% of WPA
memory
while it does almost nothing: it is run just after streaming that explicitly
frees memory that becomes unreachable. The first GGC run usually saves at
most 1% of memory and then it is never run again.
I believe this ought to also help in case we get into swap, since ltranses will
also ggc less.
Bootstrapped/regtested x86_64-linux, OK?
Hi!
I applied both patches you sent today and there are Firefox LTO -O3
results:
https://drive.google.com/file/d/0B0pisUJ80pO1ajRzLWFneTJpcE0/edit?usp=sharing
It shows that you saved a bit memory in WPA.
Martin
Honza
* lto.c (read_cgraph_and_symbols): Grow ggc memory after streaming.
* ggc.h (ggc_grow): New function.
* ggc-none.c (ggc_grow): New function.
* ggc-page.c (ggc_grow): Likewise.
Index: ggc.h
===
--- ggc.h (revision 209170)
+++ ggc.h (working copy)
@@ -225,6 +225,9 @@ extern const char *ggc_alloc_string_stat
function is called, not during allocations. */
extern void ggc_collect (void);
+/* Assume that all GGC memory is reachable and grow the limits for next collection. */
+extern void ggc_grow (void);
+
/* Register an additional root table. This can be useful for some
plugins. Does nothing if the passed pointer is NULL. */
extern void ggc_register_root_tab (const struct ggc_root_tab *);
Index: lto/lto.c
===
--- lto/lto.c (revision 209170)
+++ lto/lto.c (working copy)
@@ -2999,6 +3000,10 @@ read_cgraph_and_symbols (unsigned nfiles
gimple_canonical_types = NULL;
delete canonical_type_hash_cache;
canonical_type_hash_cache = NULL;
+
+ /* At this stage we know that majority of GGC memory is reachable.
+ Growing the limits prevents unnecesary invocation of GGC. */
+ ggc_grow ();
ggc_collect ();
/* Set the hooks so that all of the ipa passes can read in their data. */
Index: ggc-none.c
===
--- ggc-none.c (revision 209170)
+++ ggc-none.c (working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c (revision 209170)
+++ ggc-page.c (working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits for next collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long) G.allocated / 1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
Re: Avoid unnecesary GGC runs during LTO
+
+ /* At this stage we know that majority of GGC memory is reachable.
+ Growing the limits prevents unnecesary invocation of GGC. */
+ ggc_grow ();
ggc_collect ();
Isn't the collect here pointless? I see not in ENABLE_CHECKING, but
shouldn't this be abstracted away, thus call ggc_collect from ggc_grow?
Or maybe rather even for ENABLE_CHECKING adjust G.allocated_last_gc
and simply drop the ggc_collect above ().
I am fine with both. I basically decided to keep the explicit ggc_collect() to
make it clear (from lto.c source code) that we are GGC safe at this point and
to have way to double check that we do not produce too much of garbage with
checking disabled. (so with -Q I will see how much it is collected at that
place).
We can embed it into ggc_grow and document that w/o checking it is equivalent
to ggc_cooect.
Anyway, this is sth for stage1 at this point.
OK,
Honza
Thanks,
Richard.
/* Set the hooks so that all of the ipa passes can read in their data.
*/
Index: ggc-none.c
===
--- ggc-none.c (revision 209170)
+++ ggc-none.c (working copy)
@@ -63,3 +63,8 @@ ggc_free (void *p)
{
free (p);
}
+
+void
+ggc_grow (void)
+{
+}
Index: ggc-page.c
===
--- ggc-page.c (revision 209170)
+++ ggc-page.c (working copy)
@@ -2095,6 +2095,19 @@ ggc_collect (void)
fprintf (G.debug_file, END COLLECTING\n);
}
+/* Assume that all GGC memory is reachable and grow the limits for next
collection. */
+
+void
+ggc_grow (void)
+{
+#ifndef ENABLE_CHECKING
+ G.allocated_last_gc = MAX (G.allocated_last_gc,
+G.allocated);
+#endif
+ if (!quiet_flag)
+fprintf (stderr, {GC start %luk} , (unsigned long) G.allocated /
1024);
+}
+
/* Print allocation statistics. */
#define SCALE(x) ((unsigned long) ((x) 1024*10 \
? (x) \
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
