-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Fix #46 #99
base: main
Are you sure you want to change the base?
Fix #46 #99
Conversation
This is the bare minimum required to make fibers work within the go runtime.
2b4a31b
to
82ef1e8
Compare
This is not so simple, I don't know for sure if there is a solution to this problem. The stack size for goroutines is determined by the go runtime (aot, obviously), and they use split stacks. Compiling with split stack flags seemed to superficially solve the problem, but it's not reliable, I'm still getting "g0 morestack" aborts; The go runtime is creating threads that we don't have control over, probably with too small a stack size, while we could edit the thread pool library to use larger stacks we cannot exert control over go-runtime-created threads ... |
I'm on sick leave, and this is mostly about reading/research ... which I'll try to continue when able ... If anyone has any better understanding of what is going on, and if there is a solution too it, I'd be interested to hear it ... |
According to https://groups.google.com/g/golang-nuts/c/VUKub2nIt3M the stack size of the C threads can be configured using I also wonder if using the master branch of PHP (which has the |
I added it to the CFLAGS when compiling PHP: Index: Dockerfile
<+>UTF-8
===================================================================
diff --git a/Dockerfile b/Dockerfile
--- a/Dockerfile (revision 0a2e2de8428ab440b536d652d834e82e602abb7c)
+++ b/Dockerfile (date 1668063664875)
@@ -18,7 +18,7 @@
; \
\
export \
- CFLAGS="$PHP_CFLAGS" \
+ CFLAGS="$PHP_CFLAGS -fsplit-stack" \
CPPFLAGS="$PHP_CPPFLAGS" \
LDFLAGS="$PHP_LDFLAGS" \
; \ And this seems to work "in real life" (as in not getting any errors when testing on real files with a real server) but the test segfaults 100% of the time. |
Ah, I had to load test it to get it to crash. Edit: Stack size appears to be related to RPS. I set |
It appears PHP does some pretty low-level modifications to the stack when using fibers. I suppose it is possible there is some sort of bug there. |
This is all really fascinating. CGO apparently makes a copy of the stack before calling cgo as do Fibers using assembly upstreamed at Boost context. I suspect this looks something like this:
I'm nowhere near 100% certain this is what is going on, but maybe this is why increasing the stack size probably helps. |
Upon further testing (at least on x64 machine), with the Dockerfile + this change running with
With this, Frankenphp is able to quite handily handle quite a bit of connections and fibers without any crashes. Using stackusage, I was able to determine the average usage to be about 8mb on average, with a maximum of 21mb. Obviously this will be impacted by the number of Fibers created. |
I'm going to do a couple of tests with regular PHP and smaller stack size limits, since this may be a bug with PHP. Result: non-issue. |
Actually, this change causes a segfault when running the |
@withinboredom maybe could we at least patch the Docker image to have Fibers support out of the box? |
It's a specific runtime configuration, so probably just needs documentation: Docker Run: |
Alternatively, if there is some way to get PHP + golang stacks to play nice together, that would be a better solution. |
This is the bare minimum required to make fibers work within the go runtime; The go runtime uses split stacks.