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

perf: Remove all Cgo handles #1071

Closed
wants to merge 189 commits into from
Closed

Conversation

AlliBalliBaba
Copy link
Collaborator

This PR aims to improve handle performance as discussed in #934

Instead of optimizing handles, they are completely removed and replaced by passing the index of the current thread
This makes it so we can store a slice of phpThreads on the go side and have more potential control over thread
creation/destruction/metrics.
The performance impacts are similar to #934.

A quick worker benchmark:
wrk -t 4 -c 150 -d 60 http://localhost:8124 on WSL bookworm-Docker 20 CPU cores 40 threads

  • without this PR: ~57000 req/s
  • with this PR: ~65000 req/s

Note that this benchmark was done with php_import_environment_variables disabled (I plan to address it in a separate PR).

This PR comes with some necessary refactoring (especially in worker.go) so I hope it doesn't conflict too much with other potential PRs.

a.stecher and others added 30 commits September 1, 2024 23:53
# Conflicts:
#	frankenphp.go
#	go.mod
#	worker.go
Co-authored-by: Kévin Dunglas <[email protected]>
a.stecher added 28 commits September 29, 2024 13:07
# Conflicts:
#	worker.go
# Conflicts:
#	frankenphp.c
#	worker.go
# Conflicts:
#	.github/workflows/sanitizers.yaml
#	.github/workflows/tests.yaml
#	build-static.sh
#	dev-alpine.Dockerfile
#	dev.Dockerfile
#	frankenphp.go
#	static-builder.Dockerfile
#	worker.go
@AlliBalliBaba
Copy link
Collaborator Author

I'll reopen this pull request from a different branch, seems like 'main' didn't merge correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant