458316 - Reduce stack usage by allocating stack frames from heap

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Rob Borcic]

Attachment: Simple change to allocate frames from the heap rather than the stack.

Some platforms (e.g. Symbian) have a very limited amount of stack space. By default, Tamarin uses alloca to allocate interpreter stack frames. This should be configurable to allow stack frames to be allocated from the heap. This change is also required for platforms that don't have a valid implementation of alloca (e.g. Symbian).

Comment 1

[Steven Johnson]

Looks fundamentally sound, but a few comments:

(1) So I presume the ZCT usage is so that even if the Free is missed, it's still (eventually) freed? (If so, cool, good idea)

(2) Why not have the AVMPlus_PortAPI_StackAlloc/Free macros map to this instead of using explicit ifdefs? (Alternately, why not dispense with the AVMPlus_PortAPI_StackAlloc macros if we need explicit ifdefs?)

(3) It would be good to put documentation for this in avmbuild.h, e.g.

    // define this on platforms that cannot use alloca
    //#define USE_HEAP_INSTEAD_OF_STACK

and/or define it there for AVMPLUS_SYMBIAN. (Putting more such config files into common source files rather than build-specific make/project files makes things clearer for future users.)

Comment 2

[Rob Borcic]

(In reply to comment #1)

(1) The ZCT usage is so that RC objects are pinned when doing a reap. TC doesn't ref count objects in the stack frame because we pin everything that's on the C++ stack anyway. By changing the behavior to allocate from the heap, we ended up letting objects be collected too early. I had to make sure that ZCT::Reap() walks through these heap allocated stack frames just like it walks through the C++ stack.

(2) Because GC::AllocStackLikeBlock keeps a linked list of blocks there's quite a bit of overhead and we don't need this behavior everywhere that alloca is being called. In most cases where alloca is being called, we could use GC::Alloc instead. Of course, for platforms where we have a lot of stack space, using alloca has benefits, so we don't want to replace all calls to alloca with calls to GC::Alloc. I would like to replace all calls to alloca with calls to AVMPlus_PortAPI_StackAlloc and will do that in a separate patch.

(3) Following the porting API model, USE_HEAP_INSTEAD_OF_STACK would be defined in a given platform's version of portapi_avmbuild.h. I can add it (commented out) to avmbuild.h with a comment stating that its use and that it's turned off by default.

Comment 3

[Steven Johnson]

Comment on attachment 341544 [details] [diff] [review]
Simple change to allocate frames from the heap rather than the stack.

Adding Tommy to look over the ZCT changes.

Comment 4

[Steven Johnson]

OK, think I get it now. I'm a little concerned about the naming conventions we're using for the portapi; "AVMPlus_PortAPI_XXXX" seems more awkward than necessary. But this is probably something we should open another bug on, as the specific details and assumptions of portapi.h (etc) probably need some discussion to nail everything down before we go down that road all the way... opened as https://bugzilla.mozilla.org/show_bug.cgi?id=458393 so let's check this in as-is and discuss whether that's "final" form over there.

Comment 5

[Lars T Hansen]

This was resolved some time ago with the introduction of VMPI_alloca and the infrastructure surrounding it.