search

paulbartrum / jurassic

A .NET library to parse and execute JavaScript code.
MIT License
868 stars 121 forks source link

StackFrame allocation issue #200

Open jmickle66666666 opened 3 years ago

jmickle66666666 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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?