Re: RFA (GGC): PATCH to support GGC finalizers with PCH

[Richard Biener <richard dot guenther at gmail dot com>]

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
>