-
Notifications
You must be signed in to change notification settings - Fork 5
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
Increased stack-size #10
Conversation
@@ -234,6 +234,7 @@ async function main() { | |||
const nodeArgs = [ | |||
"--max-old-space-size=4096", | |||
"--expose-gc", | |||
"--stack-size=2048", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be wrong but I thought this was already the default for Node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is 984kB and I think is half of that if you're on 32bit. On PowerShell, you can see by running the command:
node --v8-options | Select-String -Pattern "stack-size" -Context 0,1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> --stack-size (default size of stack region v8 is allowed to use (in kBytes))
> type: int default: --stack-size=984
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I checked VS Code and it told me that was still the default:
--stack-size (default size of stack region v8 is allowed to use (in kBytes))
type: int default: --stack-size=984
(Set $env:ELECTRON_RUN_AS_NODE=1
and then:
& "C:\Users\jabaile\AppData\Local\Programs\Microsoft VS Code\Code.exe" --v8-options | Select-String -Pattern "stack-size" -Context 0,1
> --maglev-assert-stack-size (insert stack size checks before every IR node)
> type: bool default: --maglev-assert-stack-size
--maglev-break-on-entry (insert an int3 on maglev entries)
> --wasm-stack-switching-stack-size (default size of stacks for wasm stack-switching (in kB))
> type: int default: --wasm-stack-switching-stack-size=984
--liftoff (enable Liftoff, the baseline compiler for WebAssembly)
> --stack-size (default size of stack region v8 is allowed to use (in kBytes))
> type: int default: --stack-size=984
--max-stack-trace-source-length (maximum length of function source code printed in a stack trace.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, I don't know much about the ELECTRON_RUN_AS_NODE
flag but seems like is avoiding running the electron environment. Could that be a difference?
Do you know of another method that caused the stack trace to be processed on VSCode better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the core problem, but I don't think it would hurt to try, so long as the error deltas repo also sets this flag for consistency (does it have the same flags?).
Make sure to bump the package.json version so it gets published.
There have been some issues generated that failed due to the stack-size being too short and cannot be reproduced by Visual Studio Code editor. My suspect is that electron increases the size for 32 and 64bit to size to 4MB and 8MB respectively, avoiding the issue entirely. Reference electron_main_win.cc#L97 and BUILD.gn#L1240
Examples of issues generated by the stack trace:
microsoft/TypeScript#59678 (comment)
microsoft/TypeScript#59752 (comment)
This change increases the stack-size to 2MB, which is the minimal amount for the tests to pass.