Hi!
On 2021-08-17T09:27:46-0400, David Malcolm via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
> On Tue, 2021-08-17 at 11:17 +0200, Thomas Schwinge wrote:
>> "Turn
>> global 'ggc_force_collect' variable into 'force_collect' parameter to
>> 'ggc_collect'"
> Looks good to me, but bool params can be unclear - maybe introduce an
> enum to make the meaning more explicit to the reader of the code?
I actually had contemplated that, but then went for the simpler 'bool'
variant... ;-) But yes, it's a good suggestion, thanks. OK to push the
attached "Turn 'bool force_collect' parameter to 'ggc_collect' into an
'enum ggc_collect mode'"?
Grüße
Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From 73e4b9869e2cc515ee3393bffa220e775bbbcd45 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Tue, 17 Aug 2021 21:15:46 +0200
Subject: [PATCH] Turn 'bool force_collect' parameter to 'ggc_collect' into an
'enum ggc_collect mode'
... to make the meaning more explicit to the reader of the code.
Follow-up to recent commit 0edf2e81bb02cba43b649b3f6e7258b68a779ac0
"Turn global 'ggc_force_collect' variable into 'force_collect' parameter to
'ggc_collect'".
gcc/
* ggc.h (enum ggc_collect): New.
(ggc_collect): Use it.
* ggc-page.c: Adjust.
* ggc-common.c: Likewise.
* ggc-tests.c: Likewise.
* read-rtl-function.c: Likewise.
* selftest-run-tests.c: Likewise.
* doc/gty.texi (Invoking the garbage collector): Likewise.
Suggested-by: David Malcolm <dmalc...@redhat.com>
---
gcc/doc/gty.texi | 6 +++---
gcc/ggc-common.c | 2 +-
gcc/ggc-page.c | 5 +++--
gcc/ggc-tests.c | 18 +++++++++---------
gcc/ggc.h | 10 ++++++----
gcc/read-rtl-function.c | 2 +-
gcc/selftest-run-tests.c | 2 +-
7 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/gcc/doc/gty.texi b/gcc/doc/gty.texi
index b667d1d19ba..2ad7793191b 100644
--- a/gcc/doc/gty.texi
+++ b/gcc/doc/gty.texi
@@ -655,9 +655,9 @@ with many other garbage collectors, it is not implicitly invoked by
allocation routines when a lot of memory has been consumed. So the
only way to have GGC reclaim storage is to call the @code{ggc_collect}
function explicitly.
-When the @var{force_collect} parameter is set or otherwise an internal
-heuristic decides whether to actually collect, this call is
-potentially an expensive operation, as it may
+With @var{mode} @code{GGC_COLLECT_FORCE} or otherwise (default
+@code{GGC_COLLECT_HEURISTIC}) when the internal heuristic decides to
+collect, this call is potentially an expensive operation, as it may
have to scan the entire heap. Beware that local variables (on the GCC
call stack) are not followed by such an invocation (as many other
garbage collectors do): you should reference all your data from static
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index f38e4d5020d..32ba5be42b2 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -962,7 +962,7 @@ dump_ggc_loc_statistics ()
if (! GATHER_STATISTICS)
return;
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ggc_mem_desc.dump (GGC_ORIGIN);
}
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index a6fbecaa1d8..1c49643e7e7 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -2184,7 +2184,7 @@ validate_free_objects (void)
/* Top level mark-and-sweep routine. */
void
-ggc_collect (bool force_collect)
+ggc_collect (enum ggc_collect mode)
{
/* Avoid frequent unnecessary work by skipping collection if the
total allocations haven't expanded much since the last
@@ -2196,7 +2196,8 @@ ggc_collect (bool force_collect)
memory_block_pool::trim ();
float min_expand = allocated_last_gc * param_ggc_min_expand / 100;
- if (G.allocated < allocated_last_gc + min_expand && !force_collect)
+ if (mode == GGC_COLLECT_HEURISTIC
+ && G.allocated < allocated_last_gc + min_expand)
return;
timevar_push (TV_GC);
diff --git a/gcc/ggc-tests.c b/gcc/ggc-tests.c
index 2891c20ceac..e83f7019863 100644
--- a/gcc/ggc-tests.c
+++ b/gcc/ggc-tests.c
@@ -47,7 +47,7 @@ test_basic_struct ()
root_test_struct = ggc_cleared_alloc <test_struct> ();
root_test_struct->other = ggc_cleared_alloc <test_struct> ();
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (root_test_struct));
ASSERT_TRUE (ggc_marked_p (root_test_struct->other));
@@ -77,7 +77,7 @@ test_length ()
for (int i = 0; i < count; i++)
root_test_of_length->elem[i] = ggc_cleared_alloc <test_of_length> ();
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (root_test_of_length));
for (int i = 0; i < count; i++)
@@ -151,7 +151,7 @@ test_union ()
test_struct *referenced_by_other = ggc_cleared_alloc <test_struct> ();
other->m_ptr = referenced_by_other;
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (root_test_of_union_1));
ASSERT_TRUE (ggc_marked_p (ts));
@@ -192,7 +192,7 @@ test_finalization ()
test_struct_with_dtor::dtor_call_count = 0;
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
/* Verify that the destructor was run for each instance. */
ASSERT_EQ (count, test_struct_with_dtor::dtor_call_count);
@@ -210,7 +210,7 @@ test_deletable_global ()
test_of_deletable = ggc_cleared_alloc <test_struct> ();
ASSERT_TRUE (test_of_deletable != NULL);
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_EQ (NULL, test_of_deletable);
}
@@ -283,7 +283,7 @@ test_inheritance ()
test_some_subclass_as_base_ptr = new some_subclass ();
test_some_other_subclass_as_base_ptr = new some_other_subclass ();
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
/* Verify that the roots and everything referenced by them got marked
(both for fields in the base class and those in subclasses). */
@@ -362,7 +362,7 @@ test_chain_next ()
tail_node = new_node;
}
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
/* If we got here, we survived. */
@@ -429,7 +429,7 @@ test_user_struct ()
num_calls_to_user_gt_ggc_mx = 0;
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (root_user_struct_ptr));
ASSERT_TRUE (ggc_marked_p (referenced));
@@ -447,7 +447,7 @@ test_tree_marking ()
{
dummy_unittesting_tree = build_int_cst (integer_type_node, 1066);
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (dummy_unittesting_tree));
}
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 0f640b255e8..5e921d957fd 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -262,10 +262,12 @@ extern const char *ggc_alloc_string (const char *contents, int length
#define ggc_strdup(S) ggc_alloc_string ((S), -1 MEM_STAT_INFO)
/* Invoke the collector. Garbage collection occurs only when this
- function is called, not during allocations.
- Unless FORCE_COLLECT, an internal heuristic decides whether to actually
- collect. */
-extern void ggc_collect (bool force_collect = false);
+ function is called, not during allocations. */
+enum ggc_collect {
+ GGC_COLLECT_HEURISTIC,
+ GGC_COLLECT_FORCE
+};
+extern void ggc_collect (enum ggc_collect mode = GGC_COLLECT_HEURISTIC);
/* Return unused memory pages to the system. */
extern void ggc_trim (void);
diff --git a/gcc/read-rtl-function.c b/gcc/read-rtl-function.c
index 0badfb98f9d..941d1e158a3 100644
--- a/gcc/read-rtl-function.c
+++ b/gcc/read-rtl-function.c
@@ -1861,7 +1861,7 @@ test_loading_labels ()
/* Ensure that label names read from a dump are GC-managed
and are found through the insn. */
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
ASSERT_TRUE (ggc_marked_p (insn_200));
ASSERT_TRUE (ggc_marked_p (LABEL_NAME (insn_200)));
}
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 10881fce230..6a8f291f5dd 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -128,7 +128,7 @@ selftest::run_tests ()
issues. For example, if any GC-managed items have buggy (or missing)
finalizers, this last collection will ensure that things that were
failed to be finalized can be detected by valgrind. */
- ggc_collect (true);
+ ggc_collect (GGC_COLLECT_FORCE);
/* Finished running tests; the test_runner dtor will print a summary. */
}
--
2.30.2
