The Phobos GC presumes a linear stack that stretches from <bottom> to the current ESP.

This model is normally correct; however, the virtual stacks used in StackThreads or similar models kinda break it.

>
> Illustration:
>
>   Normal stack:
>
> [bottom]=====================================[ESP]
>
>
>   StackThreads stack:
>
> [bottom]=========|==========[End]   [Heap================================================= ... ]
>                  |
>                  +......................[Virtual stack bottom]====================[ESP]
>

This is easy to fix; introduce a new Thread function (getStackTop) that checks if a Thread member
variable (void *overrideStackTop) is set; if yes, return that variable, otherwise return getESP().

Since this is a rather smallish correction that doesn't require any API change, I'd greatly welcome it if this change was applied to 1.0, since because of the way the current GC works, writing reliable StackThreads is essentially impossible.

Thanks for your consideration,

 --downs

downs wrote:
> The Phobos GC presumes a linear stack that stretches from <bottom> to the current ESP.
> 
> This model is normally correct; however, the virtual stacks used in StackThreads or similar
> models kinda break it.
> 
>> Illustration:
>>
>>   Normal stack:
>>
>> [bottom]=====================================[ESP]
>>
>>
>>   StackThreads stack:
>>
>> [bottom]=========|==========[End]   [Heap================================================= ... ]
>>                  |
>>                  +......................[Virtual stack bottom]====================[ESP]
>>
> 
> This is easy to fix; introduce a new Thread function (getStackTop) that checks if a Thread member
> variable (void *overrideStackTop) is set; if yes, return that variable, otherwise return getESP().
> 
> Since this is a rather smallish correction that doesn't require any API change, I'd greatly welcome it
> if this change was applied to 1.0, since because of the way the current GC works, writing reliable
> StackThreads is essentially impossible.
> 
> Thanks for your consideration,
> 
>  --downs

Wouldn't this add the overhead of a function call to every collect cycle?

On Fri, 11 Jan 2008 20:36:24 +0300, Robert Fraser <fraserofthenight@gmail.com> wrote:

> downs wrote:
>> The Phobos GC presumes a linear stack that stretches from <bottom> to the current ESP.
>>  This model is normally correct; however, the virtual stacks used in StackThreads or similar
>> models kinda break it.
>>
>>> Illustration:
>>>
>>>   Normal stack:
>>>
>>> [bottom]=====================================[ESP]
>>>
>>>
>>>   StackThreads stack:
>>>
>>> [bottom]=========|==========[End]   [Heap================================================= ... ]
>>>                  |
>>>                  +.....................[Virtual stack bottom]====================[ESP]
>>>
>>  This is easy to fix; introduce a new Thread function (getStackTop) that checks if a Thread member
>> variable (void *overrideStackTop) is set; if yes, return that variable, otherwise return getESP().
>>  Since this is a rather smallish correction that doesn't require any API change, I'd greatly welcome it
>> if this change was applied to 1.0, since because of the way the current GC works, writing reliable
>> StackThreads is essentially impossible.
>>  Thanks for your consideration,
>>   --downs
>
> Wouldn't this add the overhead of a function call to every collect cycle?

Collect cycles are not so frequent for this to be a problem. Only if program allocates and frees a lot (like 1GB per second) of memory or has 1000 threads.

Robert Fraser wrote:
> Wouldn't this add the overhead of a function call to every collect cycle?

Yes, but that's hardly a problem.
Besides, if the function is made final, the compiler can inline it, bringing the cost down to a single branch.
And, as naryl said, collect runs aren't frequent enough for it to matter.
It's hardly an inner loop :)

 --downs

Attached is a patch that implements the proposed change, against GDC(Head)'s std/thread.d:

 --downs

Another problem :(

Somebody pointed out to me that mmapped files are not scanned by the GC by default. Since I use them for my virtual stacks, this naturally caused me some problems.

So I went ahead and addRanged the mmapped files.

Experimentation revealed, however, that reading these files during the GC phase
caused the OS to allocate memory for them, thus causing exactly what the use of MmFiles was
meant to prevent - an explosion of memory usage.

Thus, I want to request the addition of a way to register callbacks with the GC that are run before and after the collection phase.

This would allow me to add only the required range to scan (ESP to vstack top), thus avoiding the memory explosion problem.

 --downs

Just to prevent misunderstandings, let me clarify my position.

This patch is created against the GDC version of Phobos because it's the one I wrote it against.
But since the GDC and DMD garbage collectors are almost identical, it should apply equally to the DMD phobos source.

 --downs

PS: As of now, scrapple.tools depends on this patch being applied.
  This is necessary, because it is straight-out impossible to create StackThreads(coroutines)
  that work with an unpatched Phobos GC, without completely disabling it while stackthreads are active.
  (See first post)
    Since StackThreads are one of the core parts of scrapple.tools, this move has sadly become necessary.
  I hope this patch or functionally identical changes can eventually be merged into the Phobos repo.