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

StackFrame allocation issue #200

Open
jmickle66666666 opened this issue Mar 27, 2021 · 2 comments
Open

StackFrame allocation issue #200

jmickle66666666 opened this issue Mar 27, 2021 · 2 comments

Comments

@jmickle66666666
Copy link

It appears that PushStackFrame (ScriptEngine.cs:1350) is causing heap allocations, as StackFrame is a class and not a struct. Is it possible to make it a struct, or use object pooling instead? I'm using Jurassic for a game engine and this would greatly help performance.

@paulbartrum
Copy link
Owner

I'm pretty sure even if you make StackFrame a struct, it'll get boxed as soon as you push it on the stackFrames stack.

I do have an idea which would fix it though: instead of keeping track of stack frames manually, just rely on .NET to do it for us, and filter and transform the .NET stack trace into a JS one. This would be great for performance (zero overhead!) but involves parsing stack traces and translating IL offsets to JS line numbers, both of which are tricky.

@kpreisser
Copy link
Collaborator

kpreisser commented Jul 20, 2021

I'm pretty sure even if you make StackFrame a struct, it'll get boxed as soon as you push it on the stackFrames stack.

Actually, as stackFrames uses the generic Stack<T>, no boxing will occur as the underlying array will be of type StackFrame and therefore can hold the structs directly, instead of pointers to object instances. (Only the underlying array may need to be (re-)allocated when the number of elements to be stored would exceed the current array size.)

I think it makes sense to change StackFrame to a struct to reduce allocations (maybe also to a readonly struct to indicate it should not be modified). What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants