Re: [PATCH] Restore init_ggc_heuristics.
On Tue, Nov 19, 2019 at 2:18 PM Martin Liška wrote: > > On 11/19/19 1:42 PM, Richard Biener wrote: > > Well, then call it from the caller of init_options_struct instead, > > right after it or after the > > langhook variant is called? > > Yes, that's definitely possible, there's a patch that I've just locally > tested. > > Ready for trunk? OK. > Thanks, > Martin
Re: [PATCH] Restore init_ggc_heuristics.
On 11/19/19 1:42 PM, Richard Biener wrote: Well, then call it from the caller of init_options_struct instead, right after it or after the langhook variant is called? Yes, that's definitely possible, there's a patch that I've just locally tested. Ready for trunk? Thanks, Martin >From b868170d585354e3cfedb4b6076ec66d475fa66d Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Tue, 19 Nov 2019 13:55:40 +0100 Subject: [PATCH] Restore init_ggc_heuristics. gcc/ChangeLog: 2019-11-19 Martin Liska * toplev.c (general_init): Move the call... (toplev::main): ... here as we need init_options_struct being called. --- gcc/toplev.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gcc/toplev.c b/gcc/toplev.c index d4583bac66c..cfc757d84ba 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1240,10 +1240,6 @@ general_init (const char *argv0, bool init_signals) /* Initialize register usage now so switches may override. */ init_reg_sets (); - /* This must be done after global_init_params but before argument - processing. */ - init_ggc_heuristics (); - /* Create the singleton holder for global state. This creates the dump manager. */ g = new gcc::context (); @@ -2377,6 +2373,10 @@ toplev::main (int argc, char **argv) init_options_struct (_options, _options_set); lang_hooks.init_options_struct (_options); + /* Init GGC heuristics must be caller after we initialize + options. */ + init_ggc_heuristics (); + /* Convert the options to an array. */ decode_cmdline_options_to_array_default_mask (argc, CONST_CAST2 (const char **, -- 2.24.0
Re: [PATCH] Restore init_ggc_heuristics.
On Tue, Nov 19, 2019 at 12:37 PM Martin Liška wrote: > > On 11/19/19 11:03 AM, Richard Biener wrote: > > On Mon, Nov 18, 2019 at 1:24 PM Martin Liška wrote: > >> > >> Hello. > >> > >> After my param to option transformation, we lost automatic GGC > >> detection. It's because init_ggc_heuristics is called before > >> init_options_struct which memsets all the values to zero first. > >> > >> I've tested the patch with --enable-checking=release and I hope > >> Honza can test it more? > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > I prefer to _not_ move all the functions. Moving the init_ggc_heuristics > > call is OK. > > I would like to, but opts.o is also put into all wrappers: > > g++ -no-pie -g -DIN_GCC -fno-exceptions -fno-rtti > -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings > -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common > -DHAVE_CONFIG_H -static-libstdc++ -static-libgcc -o xgcc gcc.o gcc-main.o > ggc-none.o \ >c/gccspec.o driver-i386.o libcommon-target.a \ > libcommon.a ../libcpp/libcpp.a ../libbacktrace/.libs/libbacktrace.a > ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a > /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: > libcommon-target.a(opts.o): in function `init_options_struct(gcc_options*, > gcc_options*)': > /home/marxin/Programming/gcc/gcc/opts.c:292: undefined reference to > `init_ggc_heuristics()' > collect2: error: ld returned 1 exit status > make: *** [Makefile:2037: xgcc] Error 1 > > and adding ggc-common.o to OBJS-libcommon-target will not work. > That's why I also moved the implementation. Well, then call it from the caller of init_options_struct instead, right after it or after the langhook variant is called? Richard. > > Martin > > > > > Thanks, > > Richard. > > > >> Thanks, > >> Martin > >> > >> gcc/ChangeLog: > >> > >> 2019-11-18 Martin Liska > >> > >> * ggc-common.c (ggc_rlimit_bound): Move to opts.c > >> (ggc_min_expand_heuristic): Likewise. > >> (ggc_min_heapsize_heuristic): Likewise. > >> (init_ggc_heuristics): Likewise. > >> * ggc.h (init_ggc_heuristics): Remove declaration. > >> * opts.c (ggc_rlimit_bound): Moved here from ggc-common.c. > >> (ggc_min_expand_heuristic): Likewise. > >> (ggc_min_heapsize_heuristic): Likewise. > >> (init_ggc_heuristics): Likewise. > >> (init_options_struct): Init GGC params. > >> * toplev.c (general_init): Remove call to init_ggc_heuristics. > >> --- > >>gcc/ggc-common.c | 103 - > >>gcc/ggc.h| 3 -- > >>gcc/opts.c | 106 +++ > >>gcc/toplev.c | 4 -- > >>4 files changed, 106 insertions(+), 110 deletions(-) > >> > >> >
Re: [PATCH] Restore init_ggc_heuristics.
On 11/19/19 11:03 AM, Richard Biener wrote: On Mon, Nov 18, 2019 at 1:24 PM Martin Liška wrote: Hello. After my param to option transformation, we lost automatic GGC detection. It's because init_ggc_heuristics is called before init_options_struct which memsets all the values to zero first. I've tested the patch with --enable-checking=release and I hope Honza can test it more? Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? I prefer to _not_ move all the functions. Moving the init_ggc_heuristics call is OK. I would like to, but opts.o is also put into all wrappers: g++ -no-pie -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -static-libstdc++ -static-libgcc -o xgcc gcc.o gcc-main.o ggc-none.o \ c/gccspec.o driver-i386.o libcommon-target.a \ libcommon.a ../libcpp/libcpp.a ../libbacktrace/.libs/libbacktrace.a ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: libcommon-target.a(opts.o): in function `init_options_struct(gcc_options*, gcc_options*)': /home/marxin/Programming/gcc/gcc/opts.c:292: undefined reference to `init_ggc_heuristics()' collect2: error: ld returned 1 exit status make: *** [Makefile:2037: xgcc] Error 1 and adding ggc-common.o to OBJS-libcommon-target will not work. That's why I also moved the implementation. Martin Thanks, Richard. Thanks, Martin gcc/ChangeLog: 2019-11-18 Martin Liska * ggc-common.c (ggc_rlimit_bound): Move to opts.c (ggc_min_expand_heuristic): Likewise. (ggc_min_heapsize_heuristic): Likewise. (init_ggc_heuristics): Likewise. * ggc.h (init_ggc_heuristics): Remove declaration. * opts.c (ggc_rlimit_bound): Moved here from ggc-common.c. (ggc_min_expand_heuristic): Likewise. (ggc_min_heapsize_heuristic): Likewise. (init_ggc_heuristics): Likewise. (init_options_struct): Init GGC params. * toplev.c (general_init): Remove call to init_ggc_heuristics. --- gcc/ggc-common.c | 103 - gcc/ggc.h| 3 -- gcc/opts.c | 106 +++ gcc/toplev.c | 4 -- 4 files changed, 106 insertions(+), 110 deletions(-)
Re: [PATCH] Restore init_ggc_heuristics.
On Mon, Nov 18, 2019 at 1:24 PM Martin Liška wrote: > > Hello. > > After my param to option transformation, we lost automatic GGC > detection. It's because init_ggc_heuristics is called before > init_options_struct which memsets all the values to zero first. > > I've tested the patch with --enable-checking=release and I hope > Honza can test it more? > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? I prefer to _not_ move all the functions. Moving the init_ggc_heuristics call is OK. Thanks, Richard. > Thanks, > Martin > > gcc/ChangeLog: > > 2019-11-18 Martin Liska > > * ggc-common.c (ggc_rlimit_bound): Move to opts.c > (ggc_min_expand_heuristic): Likewise. > (ggc_min_heapsize_heuristic): Likewise. > (init_ggc_heuristics): Likewise. > * ggc.h (init_ggc_heuristics): Remove declaration. > * opts.c (ggc_rlimit_bound): Moved here from ggc-common.c. > (ggc_min_expand_heuristic): Likewise. > (ggc_min_heapsize_heuristic): Likewise. > (init_ggc_heuristics): Likewise. > (init_options_struct): Init GGC params. > * toplev.c (general_init): Remove call to init_ggc_heuristics. > --- > gcc/ggc-common.c | 103 - > gcc/ggc.h| 3 -- > gcc/opts.c | 106 +++ > gcc/toplev.c | 4 -- > 4 files changed, 106 insertions(+), 110 deletions(-) > >
Re: [PATCH] Restore init_ggc_heuristics.
On 11/18/19 2:08 PM, Jan Hubicka wrote:
You should be able to measure the difference building tramp3d on
enable-checking=release compiler. I will include the patch in my next
round of Firefox benchmark (probably tonight) unless you beat me.
I verified that by printing the values in gcc_collect:
2180 void
2181 ggc_collect (void)
2182 {
2183/* Avoid frequent unnecessary work by skipping collection if the
2184 total allocations haven't expanded much since the last
2185 collection. */
2186float allocated_last_gc =
2187 MAX (G.allocated_last_gc, (size_t)param_ggc_min_heapsize * 1024);
Martin
Re: [PATCH] Restore init_ggc_heuristics.
> Hello.
>
> After my param to option transformation, we lost automatic GGC
> detection. It's because init_ggc_heuristics is called before
> init_options_struct which memsets all the values to zero first.
>
> I've tested the patch with --enable-checking=release and I hope
> Honza can test it more?
You should be able to measure the difference building tramp3d on
enable-checking=release compiler. I will include the patch in my next
round of Firefox benchmark (probably tonight) unless you beat me.
Honza
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-11-18 Martin Liska
>
> * ggc-common.c (ggc_rlimit_bound): Move to opts.c
> (ggc_min_expand_heuristic): Likewise.
> (ggc_min_heapsize_heuristic): Likewise.
> (init_ggc_heuristics): Likewise.
> * ggc.h (init_ggc_heuristics): Remove declaration.
> * opts.c (ggc_rlimit_bound): Moved here from ggc-common.c.
> (ggc_min_expand_heuristic): Likewise.
> (ggc_min_heapsize_heuristic): Likewise.
> (init_ggc_heuristics): Likewise.
> (init_options_struct): Init GGC params.
> * toplev.c (general_init): Remove call to init_ggc_heuristics.
> ---
> gcc/ggc-common.c | 103 -
> gcc/ggc.h| 3 --
> gcc/opts.c | 106 +++
> gcc/toplev.c | 4 --
> 4 files changed, 106 insertions(+), 110 deletions(-)
>
>
> diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
> index f6e393d7bb6..86aab015b91 100644
> --- a/gcc/ggc-common.c
> +++ b/gcc/ggc-common.c
> @@ -715,109 +715,6 @@ mmap_gt_pch_use_address (void *base, size_t size, int
> fd, size_t offset)
> }
> #endif /* HAVE_MMAP_FILE */
>
> -#if !defined ENABLE_GC_CHECKING && !defined ENABLE_GC_ALWAYS_COLLECT
> -
> -/* Modify the bound based on rlimits. */
> -static double
> -ggc_rlimit_bound (double limit)
> -{
> -#if defined(HAVE_GETRLIMIT)
> - struct rlimit rlim;
> -# if defined (RLIMIT_AS)
> - /* RLIMIT_AS is what POSIX says is the limit on mmap. Presumably
> - any OS which has RLIMIT_AS also has a working mmap that GCC will use.
> */
> - if (getrlimit (RLIMIT_AS, ) == 0
> - && rlim.rlim_cur != (rlim_t) RLIM_INFINITY
> - && rlim.rlim_cur < limit)
> -limit = rlim.rlim_cur;
> -# elif defined (RLIMIT_DATA)
> - /* ... but some older OSs bound mmap based on RLIMIT_DATA, or we
> - might be on an OS that has a broken mmap. (Others don't bound
> - mmap at all, apparently.) */
> - if (getrlimit (RLIMIT_DATA, ) == 0
> - && rlim.rlim_cur != (rlim_t) RLIM_INFINITY
> - && rlim.rlim_cur < limit
> - /* Darwin has this horribly bogus default setting of
> - RLIMIT_DATA, to 6144Kb. No-one notices because RLIMIT_DATA
> - appears to be ignored. Ignore such silliness. If a limit
> - this small was actually effective for mmap, GCC wouldn't even
> - start up. */
> - && rlim.rlim_cur >= 8 * 1024 * 1024)
> -limit = rlim.rlim_cur;
> -# endif /* RLIMIT_AS or RLIMIT_DATA */
> -#endif /* HAVE_GETRLIMIT */
> -
> - return limit;
> -}
> -
> -/* Heuristic to set a default for GGC_MIN_EXPAND. */
> -static int
> -ggc_min_expand_heuristic (void)
> -{
> - double min_expand = physmem_total ();
> -
> - /* Adjust for rlimits. */
> - min_expand = ggc_rlimit_bound (min_expand);
> -
> - /* The heuristic is a percentage equal to 30% + 70%*(RAM/1GB), yielding
> - a lower bound of 30% and an upper bound of 100% (when RAM >= 1GB). */
> - min_expand /= 1024*1024*1024;
> - min_expand *= 70;
> - min_expand = MIN (min_expand, 70);
> - min_expand += 30;
> -
> - return min_expand;
> -}
> -
> -/* Heuristic to set a default for GGC_MIN_HEAPSIZE. */
> -static int
> -ggc_min_heapsize_heuristic (void)
> -{
> - double phys_kbytes = physmem_total ();
> - double limit_kbytes = ggc_rlimit_bound (phys_kbytes * 2);
> -
> - phys_kbytes /= 1024; /* Convert to Kbytes. */
> - limit_kbytes /= 1024;
> -
> - /* The heuristic is RAM/8, with a lower bound of 4M and an upper
> - bound of 128M (when RAM >= 1GB). */
> - phys_kbytes /= 8;
> -
> -#if defined(HAVE_GETRLIMIT) && defined (RLIMIT_RSS)
> - /* Try not to overrun the RSS limit while doing garbage collection.
> - The RSS limit is only advisory, so no margin is subtracted. */
> - {
> - struct rlimit rlim;
> - if (getrlimit (RLIMIT_RSS, ) == 0
> - && rlim.rlim_cur != (rlim_t) RLIM_INFINITY)
> - phys_kbytes = MIN (phys_kbytes, rlim.rlim_cur / 1024);
> - }
> -# endif
> -
> - /* Don't blindly run over our data limit; do GC at least when the
> - *next* GC would be within 20Mb of the limit or within a quarter of
> - the limit, whichever is larger. If GCC does hit the data limit,
> - compilation will fail, so this tries to be conservative. */
> - limit_kbytes = MAX (0, limit_kbytes - MAX (limit_kbytes / 4, 20 * 1024));
> -
[PATCH] Restore init_ggc_heuristics.
Hello.
After my param to option transformation, we lost automatic GGC
detection. It's because init_ggc_heuristics is called before
init_options_struct which memsets all the values to zero first.
I've tested the patch with --enable-checking=release and I hope
Honza can test it more?
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
2019-11-18 Martin Liska
* ggc-common.c (ggc_rlimit_bound): Move to opts.c
(ggc_min_expand_heuristic): Likewise.
(ggc_min_heapsize_heuristic): Likewise.
(init_ggc_heuristics): Likewise.
* ggc.h (init_ggc_heuristics): Remove declaration.
* opts.c (ggc_rlimit_bound): Moved here from ggc-common.c.
(ggc_min_expand_heuristic): Likewise.
(ggc_min_heapsize_heuristic): Likewise.
(init_ggc_heuristics): Likewise.
(init_options_struct): Init GGC params.
* toplev.c (general_init): Remove call to init_ggc_heuristics.
---
gcc/ggc-common.c | 103 -
gcc/ggc.h| 3 --
gcc/opts.c | 106 +++
gcc/toplev.c | 4 --
4 files changed, 106 insertions(+), 110 deletions(-)
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index f6e393d7bb6..86aab015b91 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -715,109 +715,6 @@ mmap_gt_pch_use_address (void *base, size_t size, int fd, size_t offset)
}
#endif /* HAVE_MMAP_FILE */
-#if !defined ENABLE_GC_CHECKING && !defined ENABLE_GC_ALWAYS_COLLECT
-
-/* Modify the bound based on rlimits. */
-static double
-ggc_rlimit_bound (double limit)
-{
-#if defined(HAVE_GETRLIMIT)
- struct rlimit rlim;
-# if defined (RLIMIT_AS)
- /* RLIMIT_AS is what POSIX says is the limit on mmap. Presumably
- any OS which has RLIMIT_AS also has a working mmap that GCC will use. */
- if (getrlimit (RLIMIT_AS, ) == 0
- && rlim.rlim_cur != (rlim_t) RLIM_INFINITY
- && rlim.rlim_cur < limit)
-limit = rlim.rlim_cur;
-# elif defined (RLIMIT_DATA)
- /* ... but some older OSs bound mmap based on RLIMIT_DATA, or we
- might be on an OS that has a broken mmap. (Others don't bound
- mmap at all, apparently.) */
- if (getrlimit (RLIMIT_DATA, ) == 0
- && rlim.rlim_cur != (rlim_t) RLIM_INFINITY
- && rlim.rlim_cur < limit
- /* Darwin has this horribly bogus default setting of
- RLIMIT_DATA, to 6144Kb. No-one notices because RLIMIT_DATA
- appears to be ignored. Ignore such silliness. If a limit
- this small was actually effective for mmap, GCC wouldn't even
- start up. */
- && rlim.rlim_cur >= 8 * 1024 * 1024)
-limit = rlim.rlim_cur;
-# endif /* RLIMIT_AS or RLIMIT_DATA */
-#endif /* HAVE_GETRLIMIT */
-
- return limit;
-}
-
-/* Heuristic to set a default for GGC_MIN_EXPAND. */
-static int
-ggc_min_expand_heuristic (void)
-{
- double min_expand = physmem_total ();
-
- /* Adjust for rlimits. */
- min_expand = ggc_rlimit_bound (min_expand);
-
- /* The heuristic is a percentage equal to 30% + 70%*(RAM/1GB), yielding
- a lower bound of 30% and an upper bound of 100% (when RAM >= 1GB). */
- min_expand /= 1024*1024*1024;
- min_expand *= 70;
- min_expand = MIN (min_expand, 70);
- min_expand += 30;
-
- return min_expand;
-}
-
-/* Heuristic to set a default for GGC_MIN_HEAPSIZE. */
-static int
-ggc_min_heapsize_heuristic (void)
-{
- double phys_kbytes = physmem_total ();
- double limit_kbytes = ggc_rlimit_bound (phys_kbytes * 2);
-
- phys_kbytes /= 1024; /* Convert to Kbytes. */
- limit_kbytes /= 1024;
-
- /* The heuristic is RAM/8, with a lower bound of 4M and an upper
- bound of 128M (when RAM >= 1GB). */
- phys_kbytes /= 8;
-
-#if defined(HAVE_GETRLIMIT) && defined (RLIMIT_RSS)
- /* Try not to overrun the RSS limit while doing garbage collection.
- The RSS limit is only advisory, so no margin is subtracted. */
- {
- struct rlimit rlim;
- if (getrlimit (RLIMIT_RSS, ) == 0
- && rlim.rlim_cur != (rlim_t) RLIM_INFINITY)
- phys_kbytes = MIN (phys_kbytes, rlim.rlim_cur / 1024);
- }
-# endif
-
- /* Don't blindly run over our data limit; do GC at least when the
- *next* GC would be within 20Mb of the limit or within a quarter of
- the limit, whichever is larger. If GCC does hit the data limit,
- compilation will fail, so this tries to be conservative. */
- limit_kbytes = MAX (0, limit_kbytes - MAX (limit_kbytes / 4, 20 * 1024));
- limit_kbytes = (limit_kbytes * 100) / (110 + ggc_min_expand_heuristic ());
- phys_kbytes = MIN (phys_kbytes, limit_kbytes);
-
- phys_kbytes = MAX (phys_kbytes, 4 * 1024);
- phys_kbytes = MIN (phys_kbytes, 128 * 1024);
-
- return phys_kbytes;
-}
-#endif
-
-void
-init_ggc_heuristics (void)
-{
-#if !defined ENABLE_GC_CHECKING && !defined ENABLE_GC_ALWAYS_COLLECT
- param_ggc_min_expand = ggc_min_expand_heuristic ();
- param_ggc_min_heapsize = ggc_min_heapsize_heuristic ();
