Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Thread.StackFrame class #1118

Merged
merged 1 commit into from May 8, 2018

Conversation

mzgoddard
Copy link
Contributor

Proposed Changes

Use a private StackFrame class to help internally manage use of Stack Frame values and memory. Create params, reported, and executionContext on demand.

Reason for Changes

params, reported and executionContext are created whether a stack frame will need them or not. Creating those members on demand in the stack frame saves time instantiating them and later garbage collecting them.

}

static release (stackFrame) {
_stackFrameFreeList.push(stackFrame.reset());

This comment was marked as abuse.

@thisandagain thisandagain added this to the May 2018 milestone May 7, 2018
Use a private StackFrame class to help internally manage use of Stack
Frame values and memory. Create params, reported, and executionContext
on demand.
Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing this works great and it makes sense to me. Just a question about the possibility of a memory leak — I'm not sure what the situation would be for frames to be continually added to the recycle bin without being removed, but seems like the kind of thing that could lead to things crashing if left running a long time?

*/
static release (stackFrame) {
if (typeof stackFrame !== 'undefined') {
_stackFrameFreeList.push(stackFrame.reset());

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@rschamp rschamp merged commit b657d6a into scratchfoundation:develop May 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants