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

Reduce runtime stack usage #36100

Open
liamappelbe opened this issue Mar 4, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@liamappelbe
Copy link
Contributor

commented Mar 4, 2019

Reduce runtime stack usage and/or reduce red zone constant, so that Dart can run using Fibers.

Suggestions from @mkustermann on #35745:

We could do several things:

a) Instead of using the constant evaluator to create the StackOverflowError exception object, we can simply allocate and canonicalized it (it's constructor is empty and there are no fields to initialize) - this avoids the recursive SOW issue described in the original comment - #35745 (comment)

b) We should make our runtime use less stack space, by e.g. not allocating over 2 KB of in-line handle stacks

c) We should more accurately measure the maximum "red zone" that the runtime needs for: throwing a StackOverflowError exception, compilations, bootstrapping the isolate, ...

Finally we should reduce the red zone constant down.

@cskau-g

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

Would it be possible to reinstate the bigger headroom we had earlier as a temporary workaround till we find the time to do the deeper investigation described above?
This would unblock the internal use case and make it all a lot less pressing.

@mkustermann

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Would it be possible to reinstate the bigger headroom

The current setting is higher than it used to as you can see in os_thread.h:

  static const intptr_t kStackSizeBuffer = (16 * KB * kWordSize);

This is 128 KB on a 64-bit system, it was raised in 30896b8 and kept this way ever since.

@mraleph mraleph assigned liamappelbe and unassigned mkustermann Mar 6, 2019

@mraleph

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

I am moving this back to @liamappelbe because it's a great bitesize project that would help to build the expertise.

@liamappelbe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

As a quick fix, I can reduce the redzone if the stack is small. Eg redzone = min(kStackSizeBuffer, 0.5 * totalStackSize)

@a-siva a-siva added the area-vm label Mar 7, 2019

@cskau-g

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2019

Do you have something I can try patch in to verify it'll work for our specific case?

Another workaround could be to make OSThread::kStackSizeBuffer a VM flag that defaults to it's current size.
This would allow us to at least configure it at VM init, which might be handy.

@mkustermann

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

Regarding my comment:

a) Instead of using the constant evaluator to create the StackOverflowError exception object, we can
simply allocate and canonicalized it (it's constructor is empty and there are no fields to initialize) - this
avoids the recursive SOW issue described in the original comment - #35745 (comment)

I've filed #36139 to track this separately (same issue caused b/127758365)

@liamappelbe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

Do you have something I can try patch in to verify it'll work for our specific case?

https://dart-review.googlesource.com/c/sdk/+/95703

dart-bot pushed a commit that referenced this issue Mar 7, 2019

Shrink the stack headroom if the stack is very small
Bug: #36100
Change-Id: I9d887e74c7f888dbeebaf02556522d375e30b97f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95703
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
@cskau-g

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Thanks, Liam.
I've patched in your change and confirmed it solves the particular internal issue.

dart-bot pushed a commit that referenced this issue May 23, 2019

Outline the StackZone's Zone
Bug: #36100
Change-Id: I17cdf438c19fcc66ceb09c23348654eb752dd3cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103560
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.