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

BrowserSubProcess - Increase X64/ARM64 stack size #3986

Closed
2 tasks done
amaitland opened this issue Feb 1, 2022 · 6 comments
Closed
2 tasks done

BrowserSubProcess - Increase X64/ARM64 stack size #3986

amaitland opened this issue Feb 1, 2022 · 6 comments
Assignees
Milestone

Comments

@amaitland
Copy link
Member

amaitland commented Feb 1, 2022

Reports suggest DevTools requires a larger stack size

https://bitbucket.org/chromiumembedded/cef/issues/3250/devtools-crash-when-inspecting-deeply#comment-61774526

Check chrome.exe stack size for reference

https://myprogrammingnotes.com/check-set-stack-size-limit-qt-applications.html
https://bitbucket.org/chromiumembedded/cef/issues/3252/nested-hang-on-cefsimple#comment-61787323

  • Validate stack size for both arm64 and x64
  • Validate strong name after stack size change
@amaitland amaitland self-assigned this Feb 1, 2022
@amaitland
Copy link
Member Author

if (current_cpu == "x86") {
        # Set the initial stack size to 0.5MiB, instead of the 1.5MiB needed by
        # Chrome's main thread. This saves significant memory on threads (like
        # those in the Windows thread pool, and others) whose stack size we can
        # only control through this setting. Because Chrome's main thread needs
        # a minimum 1.5 MiB stack, the main thread (in 32-bit builds only) uses
        # fibers to switch to a 1.5 MiB stack before running any other code.
        ldflags = [ "/STACK:0x80000" ]
      } else {
        # Increase the initial stack size. The default is 1MB, this is 8MB.
        ldflags = [ "/STACK:0x800000" ]
      }

@amaitland amaitland added this to the 99.2.x milestone Mar 1, 2022
@amaitland
Copy link
Member Author

The stack size for x64 will be updated first as it's a straight forward change.

Both project files will need to be updated:

The netcore project doesn't have a editbin entry, have to double check as to the reason that wasn't included.

amaitland added a commit that referenced this issue Mar 1, 2022
- For Net 4.5.2 it appears we need to resign the exe
- For .Net Core as the exes are native it doesn't look
 like we need to sign them (the CefSharp.BrowserSubprocess.dll is actually
 the one that's signed).
- Remove unused CefSharpBrowserSubprocessPostBuildEvent

Issue #3986
@amaitland
Copy link
Member Author

x64 and arm64 should both now have an 8mb stack size. Will need to validate both before release as arm64 have stopped working for me locally after installing VS2022.

@amaitland amaitland changed the title BrowserSubProcess - Investigate increasing stack size BrowserSubProcess - Increase stack size Mar 1, 2022
@amaitland amaitland modified the milestones: 99.2.x, 100.0.x Mar 7, 2022
@amaitland amaitland changed the title BrowserSubProcess - Increase stack size BrowserSubProcess - Increase X64/ARM64 stack size Apr 4, 2022
@amaitland
Copy link
Member Author

Renaming issue as I'm not planning on increasing the x86 stack size as part of this issue.

@amaitland
Copy link
Member Author

The exes in the 100.0.120-pre release appear to have their values set correctly. They're all TSAware, LargeAddressAware for the x86 builds, the .Net 4.5.2 exes are strongly named. The .Net Core exes aren't (which is expected as it's a bootstrapper exe for the dll of same name). The .Net Core x86 exe appears to already have a slightly bigger stack size.

Closing as this appears to be complete.

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

1 participant