* RFA (GGC): PATCH to support GGC finalizers with PCH
@ 2015-11-17 14:09 Jason Merrill
2015-11-17 14:39 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2015-11-17 14:09 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 779 bytes --]
While I was looking at the interaction of delayed folding with GGC, I
noticed that ggc_handle_finalizers currently runs no finalizers if
G.context_depth != 0. So any GC objects in a greater depth will still
be collected, but they won't have their finalizers run. This
specifically affects compiles that use a PCH file, since G.context_depth
is set to 1 after loading the PCH.
This patch fixes ggc_handle_finalizers to look at the depth of each
finalizer so that we still don't try to run finalizers for
non-collectable objects loaded from the PCH, but we do run finalizers
for collectable objects allocated after loading the PCH.
I ended up not relying on this for delayed folding, but it still seems
like a good bug fix.
Tested x86_64-pc-linux-gnu. OK for trunk?
[-- Attachment #2: ggc-depth.patch --]
[-- Type: text/x-patch, Size: 3208 bytes --]
commit 0bd746ae39b37b9b08e4d861d97fe30ecf4e8ad8
Author: Jason Merrill <jason@redhat.com>
Date: Fri Nov 13 09:39:15 2015 -0500
Support GGC finalizers with PCH.
* ggc-page.c (class finalizer): Add m_depth field.
(finalizer::finalizer): Initialize it.
(finalizer::depth): Return it.
(class vec_finalizer): Likewise.
(ggc_internal_alloc): Adjust constructor calls.
(ggc_handle_finalizers): Run finalizers that are deep enough.
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index deb21bb..1d5aeef 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -331,22 +331,26 @@ typedef struct page_table_chain
class finalizer
{
public:
- finalizer (void *addr, void (*f)(void *)) : m_addr (addr), m_function (f) {}
+ finalizer (void *addr, void (*f)(void *), unsigned short depth)
+ : m_addr (addr), m_function (f), m_depth(depth) {}
void *addr () const { return m_addr; }
-
+ unsigned short depth () const { return m_depth; }
void call () const { m_function (m_addr); }
private:
void *m_addr;
void (*m_function)(void *);
+ unsigned short m_depth;
};
class vec_finalizer
{
public:
- vec_finalizer (uintptr_t addr, void (*f)(void *), size_t s, size_t n) :
- m_addr (addr), m_function (f), m_object_size (s), m_n_objects (n) {}
+ vec_finalizer (uintptr_t addr, void (*f)(void *), size_t s, size_t n,
+ unsigned short depth)
+ : m_addr (addr), m_function (f), m_object_size (s), m_n_objects (n),
+ m_depth (depth) {}
void call () const
{
@@ -355,13 +359,15 @@ public:
}
void *addr () const { return reinterpret_cast<void *> (m_addr); }
+ unsigned short depth () const { return m_depth; }
private:
uintptr_t m_addr;
void (*m_function)(void *);
size_t m_object_size;
size_t m_n_objects;
- };
+ unsigned short m_depth;
+};
#ifdef ENABLE_GC_ALWAYS_COLLECT
/* List of free objects to be verified as actually free on the
@@ -1388,10 +1394,11 @@ ggc_internal_alloc (size_t size, void (*f)(void *), size_t s, size_t n
timevar_ggc_mem_total += object_size;
if (f && n == 1)
- G.finalizers.safe_push (finalizer (result, f));
+ G.finalizers.safe_push (finalizer (result, f, G.context_depth));
else if (f)
G.vec_finalizers.safe_push
- (vec_finalizer (reinterpret_cast<uintptr_t> (result), f, s, n));
+ (vec_finalizer (reinterpret_cast<uintptr_t> (result), f, s, n,
+ G.context_depth));
if (GATHER_STATISTICS)
{
@@ -1875,14 +1882,12 @@ clear_marks (void)
static void
ggc_handle_finalizers ()
{
- if (G.context_depth != 0)
- return;
-
unsigned length = G.finalizers.length ();
for (unsigned int i = 0; i < length;)
{
finalizer &f = G.finalizers[i];
- if (!ggc_marked_p (f.addr ()))
+ if (f.depth() >= G.context_depth
+ && !ggc_marked_p (f.addr ()))
{
f.call ();
G.finalizers.unordered_remove (i);
@@ -1897,7 +1902,8 @@ ggc_handle_finalizers ()
for (unsigned int i = 0; i < length;)
{
vec_finalizer &f = G.vec_finalizers[i];
- if (!ggc_marked_p (f.addr ()))
+ if (f.depth() >= G.context_depth
+ && !ggc_marked_p (f.addr ()))
{
f.call ();
G.vec_finalizers.unordered_remove (i);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA (GGC): PATCH to support GGC finalizers with PCH
2015-11-17 14:09 RFA (GGC): PATCH to support GGC finalizers with PCH Jason Merrill
@ 2015-11-17 14:39 ` Richard Biener
2015-11-17 19:46 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2015-11-17 14:39 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Tue, Nov 17, 2015 at 3:09 PM, Jason Merrill <jason@redhat.com> wrote:
> While I was looking at the interaction of delayed folding with GGC, I
> noticed that ggc_handle_finalizers currently runs no finalizers if
> G.context_depth != 0. So any GC objects in a greater depth will still be
> collected, but they won't have their finalizers run. This specifically
> affects compiles that use a PCH file, since G.context_depth is set to 1
> after loading the PCH.
>
> This patch fixes ggc_handle_finalizers to look at the depth of each
> finalizer so that we still don't try to run finalizers for non-collectable
> objects loaded from the PCH, but we do run finalizers for collectable
> objects allocated after loading the PCH.
>
> I ended up not relying on this for delayed folding, but it still seems like
> a good bug fix.
>
> Tested x86_64-pc-linux-gnu. OK for trunk?
Hmm, this enlarges finalizer/vec_finalizer. Wouldn't it be better to
add separate finalizer vectors for context_depth != 0? (I'm proposing
to add one for exactly context_depth == 1)
When is context_depth increased other than for PCH?
Richard.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA (GGC): PATCH to support GGC finalizers with PCH
2015-11-17 14:39 ` Richard Biener
@ 2015-11-17 19:46 ` Jason Merrill
2015-11-18 9:30 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2015-11-17 19:46 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]
On 11/17/2015 09:39 AM, Richard Biener wrote:
> On Tue, Nov 17, 2015 at 3:09 PM, Jason Merrill <jason@redhat.com> wrote:
>> While I was looking at the interaction of delayed folding with GGC, I
>> noticed that ggc_handle_finalizers currently runs no finalizers if
>> G.context_depth != 0. So any GC objects in a greater depth will still be
>> collected, but they won't have their finalizers run. This specifically
>> affects compiles that use a PCH file, since G.context_depth is set to 1
>> after loading the PCH.
>>
>> This patch fixes ggc_handle_finalizers to look at the depth of each
>> finalizer so that we still don't try to run finalizers for non-collectable
>> objects loaded from the PCH, but we do run finalizers for collectable
>> objects allocated after loading the PCH.
>>
>> I ended up not relying on this for delayed folding, but it still seems like
>> a good bug fix.
>>
>> Tested x86_64-pc-linux-gnu. OK for trunk?
>
> Hmm, this enlarges finalizer/vec_finalizer. Wouldn't it be better to
> add separate finalizer vectors for context_depth != 0? (I'm proposing
> to add one for exactly context_depth == 1)
>
> When is context_depth increased other than for PCH?
That seems to be the only place it's changed currently. I was assuming
that the generalized way ggc-page handles context_depth was intended to
support more depths in the future (perhaps for collecting after
processing a nested function?), so my patch was following that model.
How about this?
Jason
[-- Attachment #2: ggc-depth.patch --]
[-- Type: text/x-patch, Size: 4510 bytes --]
commit afb196cd7fc176736f9ff2abf92690a7c4ae4f94
Author: Jason Merrill <jason@redhat.com>
Date: Fri Nov 13 09:39:15 2015 -0500
Support GGC finalizers with PCH.
* ggc-page.c (ggc_globals): Change finalizers and vec_finalizers
to be vecs of vecs.
(add_finalizer): Split out from ggc_internal_alloc.
(ggc_handle_finalizers): Run finalizers for the current depth.
(init_ggc, ggc_pch_read): Reserve space for finalizers.
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index deb21bb..c3af3c8 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -361,7 +361,7 @@ private:
void (*m_function)(void *);
size_t m_object_size;
size_t m_n_objects;
- };
+};
#ifdef ENABLE_GC_ALWAYS_COLLECT
/* List of free objects to be verified as actually free on the
@@ -456,11 +456,11 @@ static struct ggc_globals
better runtime data access pattern. */
unsigned long **save_in_use;
- /* Finalizers for single objects. */
- vec<finalizer> finalizers;
+ /* Finalizers for single objects. The first index is collection_depth. */
+ vec<vec<finalizer> > finalizers;
/* Finalizers for vectors of objects. */
- vec<vec_finalizer> vec_finalizers;
+ vec<vec<vec_finalizer> > vec_finalizers;
#ifdef ENABLE_GC_ALWAYS_COLLECT
/* List of free objects to be verified as actually free on the
@@ -1240,6 +1240,23 @@ ggc_round_alloc_size (size_t requested_size)
return size;
}
+static void
+add_finalizer (void *result, void (*f)(void *), size_t s, size_t n)
+{
+ if (f == NULL)
+ ;
+ else if (n == 1)
+ {
+ finalizer fin (result, f);
+ G.finalizers[G.context_depth].safe_push (fin);
+ }
+ else
+ {
+ vec_finalizer fin (reinterpret_cast<uintptr_t> (result), f, s, n);
+ G.vec_finalizers[G.context_depth].safe_push (fin);
+ }
+}
+
/* Allocate a chunk of memory of SIZE bytes. Its contents are undefined. */
void *
@@ -1387,11 +1404,8 @@ ggc_internal_alloc (size_t size, void (*f)(void *), size_t s, size_t n
/* For timevar statistics. */
timevar_ggc_mem_total += object_size;
- if (f && n == 1)
- G.finalizers.safe_push (finalizer (result, f));
- else if (f)
- G.vec_finalizers.safe_push
- (vec_finalizer (reinterpret_cast<uintptr_t> (result), f, s, n));
+ if (f)
+ add_finalizer (result, f, s, n);
if (GATHER_STATISTICS)
{
@@ -1788,6 +1802,9 @@ init_ggc (void)
G.by_depth_max = INITIAL_PTE_COUNT;
G.by_depth = XNEWVEC (page_entry *, G.by_depth_max);
G.save_in_use = XNEWVEC (unsigned long *, G.by_depth_max);
+
+ G.finalizers.safe_grow_cleared (1);
+ G.vec_finalizers.safe_grow_cleared (1);
}
/* Merge the SAVE_IN_USE_P and IN_USE_P arrays in P so that IN_USE_P
@@ -1875,36 +1892,42 @@ clear_marks (void)
static void
ggc_handle_finalizers ()
{
- if (G.context_depth != 0)
- return;
-
- unsigned length = G.finalizers.length ();
- for (unsigned int i = 0; i < length;)
+ unsigned dlen = G.finalizers.length();
+ for (unsigned d = G.context_depth; d < dlen; ++d)
{
- finalizer &f = G.finalizers[i];
- if (!ggc_marked_p (f.addr ()))
+ vec<finalizer> &v = G.finalizers[d];
+ unsigned length = v.length ();
+ for (unsigned int i = 0; i < length;)
{
- f.call ();
- G.finalizers.unordered_remove (i);
- length--;
+ finalizer &f = v[i];
+ if (!ggc_marked_p (f.addr ()))
+ {
+ f.call ();
+ v.unordered_remove (i);
+ length--;
+ }
+ else
+ i++;
}
- else
- i++;
}
-
- length = G.vec_finalizers.length ();
- for (unsigned int i = 0; i < length;)
+ gcc_assert (dlen == G.vec_finalizers.length());
+ for (unsigned d = G.context_depth; d < dlen; ++d)
{
- vec_finalizer &f = G.vec_finalizers[i];
- if (!ggc_marked_p (f.addr ()))
+ vec<vec_finalizer> &vv = G.vec_finalizers[d];
+ unsigned length = vv.length ();
+ for (unsigned int i = 0; i < length;)
{
- f.call ();
- G.vec_finalizers.unordered_remove (i);
- length--;
+ vec_finalizer &f = vv[i];
+ if (!ggc_marked_p (f.addr ()))
+ {
+ f.call ();
+ vv.unordered_remove (i);
+ length--;
+ }
+ else
+ i++;
}
- else
- i++;
}
}
@@ -2545,6 +2568,8 @@ ggc_pch_read (FILE *f, void *addr)
pages to be 1 too. PCH pages will have depth 0. */
gcc_assert (!G.context_depth);
G.context_depth = 1;
+ G.finalizers.safe_grow_cleared (2);
+ G.vec_finalizers.safe_grow_cleared (2);
for (i = 0; i < NUM_ORDERS; i++)
{
page_entry *p;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFA (GGC): PATCH to support GGC finalizers with PCH
2015-11-17 19:46 ` Jason Merrill
@ 2015-11-18 9:30 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-11-18 9:30 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Tue, Nov 17, 2015 at 8:46 PM, Jason Merrill <jason@redhat.com> wrote:
> On 11/17/2015 09:39 AM, Richard Biener wrote:
>>
>> On Tue, Nov 17, 2015 at 3:09 PM, Jason Merrill <jason@redhat.com> wrote:
>>>
>>> While I was looking at the interaction of delayed folding with GGC, I
>>> noticed that ggc_handle_finalizers currently runs no finalizers if
>>> G.context_depth != 0. So any GC objects in a greater depth will still be
>>> collected, but they won't have their finalizers run. This specifically
>>> affects compiles that use a PCH file, since G.context_depth is set to 1
>>> after loading the PCH.
>>>
>>> This patch fixes ggc_handle_finalizers to look at the depth of each
>>> finalizer so that we still don't try to run finalizers for
>>> non-collectable
>>> objects loaded from the PCH, but we do run finalizers for collectable
>>> objects allocated after loading the PCH.
>>>
>>> I ended up not relying on this for delayed folding, but it still seems
>>> like
>>> a good bug fix.
>>>
>>> Tested x86_64-pc-linux-gnu. OK for trunk?
>>
>>
>> Hmm, this enlarges finalizer/vec_finalizer. Wouldn't it be better to
>> add separate finalizer vectors for context_depth != 0? (I'm proposing
>> to add one for exactly context_depth == 1)
>>
>> When is context_depth increased other than for PCH?
>
>
> That seems to be the only place it's changed currently. I was assuming that
> the generalized way ggc-page handles context_depth was intended to support
> more depths in the future (perhaps for collecting after processing a nested
> function?), so my patch was following that model.
>
> How about this?
Looks good apart from the .safe_grow_cleared () which should probably
better do a .safe_push (vNULL)? ("cleared" exposes too much of an
implementation detail for vec<>)
Thanks,
Richard.
>
> Jason
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-18 9:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-17 14:09 RFA (GGC): PATCH to support GGC finalizers with PCH Jason Merrill
2015-11-17 14:39 ` Richard Biener
2015-11-17 19:46 ` Jason Merrill
2015-11-18 9:30 ` Richard Biener
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).