perf: Put all $_SERVER vars into one function call. #1303
Conversation
|
My only issue with this is the loss of flexibility and cost of change. In other words, it locks us into a single way of doing things. Otherwise, it's a good idea. |
|
@withinboredom as it's purely internal, we can change that back when/if needed. |
| @@ -316,6 +314,7 @@ func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ ca | |||
| documentRootOption, | |||
| frankenphp.WithRequestSplitPath(f.SplitPath), | |||
| frankenphp.WithRequestPreparedEnv(env), | |||
| frankenphp.WithOriginalRequest(&origReq), | |||
There was a problem hiding this comment.
Can't we keep the previous strategy to not increase the public API surface?
There was a problem hiding this comment.
I'll try passing this via context instead. Doing it via the prepared env requires copying and registering it on every request (and passing the request uri via environment is a bit confusing)
There was a problem hiding this comment.
Ok, using the context is a good idea.
There was a problem hiding this comment.
I realized that you actually can't pass via context between packages, so this would need to be in the public api
There was a problem hiding this comment.
I don't think it's necessarily bad to have this in a public api since having a different 'actual' request uri is a key PHP behavior for any application using index files. We can also only pass the uri instead of the whole request.
If you still dislike that, we can also reset it back to be passed via env (feels hacky but minimizes the public api)
|
Thank you!! Great work as usual. |
After seeing that Cgo call overhead from entersyscall and exitsyscall is not insignificant, I though we might as well go all the way...
With many Cgo Calls
With one big ugly Cgo Call and the original REQUEST_URI not in the prepared env (~3.5% faster on Linux)
The PR also passes the original request uri directly instead of via the prepared env so the preparedEnv doesn't need to be copied on each request (I was confused where the original request uri was actually coming from).