Closed eamsden closed 11 months ago
I should have read this issue before looking at the PR. After much confusion, @eamsden explained this to me while I was reviewing #175 and I'm on board with it.
As @eamsden points out in the issue, and as @drbeefsupreme noted during the review for #175, we'll need to either audit the code for "top is west frame" assumptions, and we should keep this change in mind during future debugging efforts, when and if memory errors appear. (Maybe make a "likely candidates" playbook to document potential bug-causing PRs?).
This was fixed in #178 after being merged and reverted in #175 and #177
Currently in #143 we hard-reset the stack after each event.. This wipes out our hot and warm states, which are not saved to the snapshot as they contain function pointers. Thus, we have to re-initialize both after every event. Saving the state of the stack and restoring to the saved state suffices to preserve the hot state, which should not change over the course of our execution. However, it is not sufficient to save the warm state, which is reinitialized whenever the cold state changes and thus requires collection of stale entries and HAMT stems.
Instead we should implement a top-frame copying GC as follows:
The effect of this is that each event will run with the opposite-orientation frame as top each time. This should be fine, though we should audit for assumptions that "the top is always west." If the top frame has no locals and wasn't using the lightweight stack when we pushed, then this wastes no space. However if for some odd reason we did use locals or the lightweight stack in the top frame, the wasted space is now just part of the allocation area for the new "top" frame, and will be reclaimed when we switch back next time.
@joemfb @frodwith @ashelkovnykov please review this idea.