public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Avoid unnecesary GGC runs during LTO
@ 2014-04-11  6:08 Jan Hubicka
  2014-04-11  7:58 ` Richard Biener
  2014-04-11  8:59 ` Martin Liška
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Hubicka @ 2014-04-11  6:08 UTC (permalink / raw)
  To: gcc-patches, rguenther


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 ();
 
   /* 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) \

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-11  6:08 Avoid unnecesary GGC runs during LTO Jan Hubicka
@ 2014-04-11  7:58 ` Richard Biener
  2014-04-11 19:14   ` Jan Hubicka
  2014-04-11  8:59 ` Martin Liška
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Biener @ 2014-04-11  7:58 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

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 <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-11  6:08 Avoid unnecesary GGC runs during LTO Jan Hubicka
  2014-04-11  7:58 ` Richard Biener
@ 2014-04-11  8:59 ` Martin Liška
  1 sibling, 0 replies; 8+ messages in thread
From: Martin Liška @ 2014-04-11  8:59 UTC (permalink / raw)
  To: gcc-patches

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) \

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-11  7:58 ` Richard Biener
@ 2014-04-11 19:14   ` Jan Hubicka
  2014-04-17 16:07     ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2014-04-11 19:14 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

> > +
> > +  /* 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 <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-11 19:14   ` Jan Hubicka
@ 2014-04-17 16:07     ` Jan Hubicka
  2014-04-17 17:18       ` Richard Biener
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2014-04-17 16:07 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Richard Biener, gcc-patches

> > > +
> > > +  /* 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 <rguenther@suse.de>
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-17 16:07     ` Jan Hubicka
@ 2014-04-17 17:18       ` Richard Biener
  2014-04-17 17:29         ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2014-04-17 17:18 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On April 17, 2014 6:03:13 PM CEST, Jan Hubicka <hubicka@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 <rguenther@suse.de>
>> > SUSE / SUSE Labs
>> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
>> > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-17 17:18       ` Richard Biener
@ 2014-04-17 17:29         ` Jan Hubicka
  2014-04-17 18:33           ` Richard Biener
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2014-04-17 17:29 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

> On April 17, 2014 6:03:13 PM CEST, Jan Hubicka <hubicka@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 <rguenther@suse.de>
> >> > SUSE / SUSE Labs
> >> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> >> > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Avoid unnecesary GGC runs during LTO
  2014-04-17 17:29         ` Jan Hubicka
@ 2014-04-17 18:33           ` Richard Biener
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Biener @ 2014-04-17 18:33 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On April 17, 2014 7:18:05 PM CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On April 17, 2014 6:03:13 PM CEST, Jan Hubicka <hubicka@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 <rguenther@suse.de>
>> >> > SUSE / SUSE Labs
>> >> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
>> >> > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
>> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-17 18:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-11  6:08 Avoid unnecesary GGC runs during LTO Jan Hubicka
2014-04-11  7:58 ` Richard Biener
2014-04-11 19:14   ` Jan Hubicka
2014-04-17 16:07     ` Jan Hubicka
2014-04-17 17:18       ` Richard Biener
2014-04-17 17:29         ` Jan Hubicka
2014-04-17 18:33           ` Richard Biener
2014-04-11  8:59 ` Martin Liška

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).