Rewrite Stack to be double stack

[tsholmes]

Stack operations had to shift everything in the above stack up and down on every push and pop. The above stack is usually small since it is just the context chain, but the extra shuffling adds up. This switches it to be two separate stacks (implemented as fixed-size arrays to avoid re-allocating), changing the MoveStackPointer into PushAbove and PopAbove, as it was never used for transferring between the stacks, just for accessing the above stack.

This reduces the runtime of my sort script from ~16 seconds to ~14.5 seconds.

[Dunbaratu]

Just a few suggestions from my reading tonight. I'm not done, but I need to quit for the night. If you want to react to these and make further edits, feel free.

[Dunbaratu]

We should stop calling this "above".
If we're changing this into two different stacks, I think it we should also change the terminology.
This was called the "above" stack purely because of where it was located - above the other stack.
This terminology change should also affect all the other places the word "above" is used to refer to this stack, like PopAbove, PushAbove, aboveCount, etc. There may also be mentions of this in comments trying to explain the "above stack", which should change too.

Ideas for new term: scopeStack, or callStack.

[Dunbaratu]

Note to self: this will be a source of merge conflict for me: On the subject of creating the stack so it stays permanently at MAX_STACK_SIZE all the time - I had a change in my clock cycle branch designed to do this already but it wasn't ready to post it - I was keeping it as a stack (not an array) but telling .net to pre-allocate the stack to the full capacity. I will need to take care when updating that branch to develop because this will conflict for sure.

[tsholmes]

ah yeah good point. i was just trying to keep things as similar as possible, but some clearer naming here would definitely make this more approachable in the future.

[Dunbaratu]

If we're changing the underlying implementation, I don't mind exposing that fact to users in the Dump() output. This is meant to be a dump of what is really going on under the hood, so we should just print out the two stacks as two different stacks, labeled accordingly.

[Dunbaratu]

I like moving this out - it was getting nested a bit too deep.

[Dunbaratu]

A checklist of things I'm going to edit in my review - just to make sure I get all of them:

• We should have two different MAX_STACK_SIZE values now, one for each stack, just in case we feel like setting their limits differently from each other later.
• Instead of peeking a negative number to get to the scope stack, explicitly provide separate scope stack peeking API that uses positive numbers, and throw a "you should never see this" exception if we still pass in a negative number somewhere. This change also has to propagate upward through the CPU and ICPU wrappers around the stack methods providing separate methods there too.
• Instead of "scope stack" and "stack", rename the "stack" to "argument stack" so both stacks have descriptive names. This includes:
  • Rename them inside Stack.cs and IStack.cs, and renaming all the API methods accordingly.
  • Rename their wrapper methods inside CPU.cs and ICPU.cs to give the same effect.
• Some comments still refer to the way the "above stack" worked. They are easy to miss because they're comments so don't affect compilation. Edit comments to explain things the new way.