GC Crash/Bug due recursion?

> On Fri, Jun 10, 2011 at 16:15, Holger Hans Peter Freyther
> <[hidden email]>  wrote:
>> On 06/10/2011 04:11 PM, Paolo Bonzini wrote:
>>
>>> Looks like an infinite loop in Smalltalk... Do you have enough stack
>>> space to do a "call _gst_show_backtrace(stdout)" from gdb?
>>
>> Hi,
>>
>> yes, I just wonder if things should crash or if this can be prevented, it
>> seems to create an infinite loop in the GC which is weird... of course with
>> recursion in smalltalk the stack will be full too..
>
> Yes, I think the problem is that the C stack of the marker is
> exhausted before the whole Smalltalk stack is visited.
>
> Paolo
>
> _______________________________________________
> help-smalltalk mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/help-smalltalk

> On 06/21/2011 12:00 AM, Gwenael Casaccio wrote:
>>
>> This bench stresses well the it. mark patch and crashes the rec. mark.
>> But I guess there are still bugs with contexts (I should take a look),
>> there is a strange crash (DNU with #printNl lookup symbol is not found
>> but it's present...). But that's a good start :-)
>>
>> It's possible instead of the iterative version to use a mark with a
>> pointer reversal but it's harder to debug, requires high time-cost:
>> visits each branch-node at least (n+1) times, each visit requires
>> additional memory fetches, and each visit cycles 4 values +
>> reading/writing mark-flags.
>>
>> Another solution is to use a stack guard (v8 does it) and checks
>> overflow Knuth propose something similar
>
> Modulo the usual comments about irrelevant hunks :) it looks good,
> thanks! Can you however ensure the invariant that the queue is empty
> upon entry to _gst_mark, by making it accept an OOP? I believe this
> simplifies the code and lets you add back the tail recursion
> optimization you removed.
>
> And it would be even better if you could keep the distinction between
> marking one OOP and marking a range, because it removes a lot of traffic
> to/from the mark queue (a bit cheaper than the stack because of no stack
> frame overhead, but still expensive cache-wise). Which in turn means,
> keep the API as is! :)
>
> Anyway, good start!
>
> Paolo

> On 07/04/2011 12:48 PM, Gwenael Casaccio wrote:
>> Is it better Paolo ?
>
> What's the difference from v1?  I see nothing of what I asked for:
>
>>> Modulo the usual comments about irrelevant hunks :) it looks good,
>>> thanks! Can you however ensure the invariant that the queue is empty
>>> upon entry to _gst_mark, by making it accept an OOP? I believe this
>>> simplifies the code and lets you add back the tail recursion
>>> optimization you removed.
>>>
>>> And it would be even better if you could keep the distinction between
>>> marking one OOP and marking a range, because it removes a lot of traffic
>>> to/from the mark queue (a bit cheaper than the stack because of no stack
>>> frame overhead, but still expensive cache-wise). Which in turn means,
>>> keep the API as is! :)
>
> Paolo

> What I asked:
>
>>>> by making it accept an OOP
>
> What you have:
>
>> --- a/libgst/oop.c
>> +++ b/libgst/oop.c
>> @@ -2242,68 +2242,89 @@ _gst_add_an_oop_to_mark_queue (OOP oop)
>> void
>> _gst_mark (void)
>
> What I asked:
>
>>> What's the difference from v1?
>
> What you have:
>
>> Sorry it was not the good patch.
>
> Please read messages _before_ replying to them.
>
> Paolo

> On 07/04/2011 09:42 PM, Gwenael Casaccio wrote:
>>
>> I've made some changes:
>>
>> 1) I keep _gst_mark_an_oop_internal and removed
>> _gst_add_an_oop_to_mark_queue for adding an OOP in the Queue. So like
>> the previous one, it has an OOP has argument and visit it.
>>
>> 2) in _gst_mark_an_oop_internal when visiting one OOP I keep the same
>> behavior with TAIL_MARK_OOP, and TAIL_MARK_OOPRANGE add OOPs in the
>> Queue.
>>
>> 3) _gst_mark_oop_range keeps the same behavior as the old one
>>
>> The patch is tested and produced with master (all tests are green)
>
> Thanks, these are the minimum changes required. I'll tweak it a bit to
> remove the final push/pop and commit it.