perf: Remove all Cgo handles

[AlliBalliBaba]

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 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 optimize that it in a separate PR).

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

[AlliBalliBaba]

I realized that you can also create go flamegraphs with go tool pprof. I'm creating them like this in the dev.Dockerfile:

# installation
mkdir /usr/local/src/flamegraph && \
cd /usr/local/src/flamegraph && \
git clone https://github.com/brendangregg/FlameGraph.git && \
cd /usr/local/src/flamegraph/FlameGraph

# create a 20s flamegraph.svg in project dir
go tool pprof -raw -output=cpu.txt 'http://localhost:2019/debug/pprof/profile?seconds=20' && \
./stackcollapse-go.pl cpu.txt | ./flamegraph.pl > /go/src/app/flame.svg

Flamegraph after this PR:

(M12 comes from import_environment_variables and scales linearly with the amount of environment variables)

[AlliBalliBaba]

@withinboredom I also noticed that with the current settings of minWorkerErrorBackoff=10ms the failing-worker.php test will sometimes crash due to consuming too much memory ('core dump', also in the main branch).

Changing this to 100ms fixes the issue. Still it's weird that it would start consuming so much memory, maybe it's spawning too many goroutines?

[dunglas]

Awesome! Very good idea.

[dunglas]

Shouldn't we use uint64_t or uint32_t everywhere? (C-side and Go side)? The current implementation may trigger incompatibilities between C and Go types if a (very) large number of threads is started.

Suggested change

	func go_handle_request(threadIndex int) bool {
	func go_handle_request(threadIndex C.uint64_t) bool {

[AlliBalliBaba]

I was wondering how go treats the integers coming from C since they don't seem to need casting. I changed it to uint32_t since I'm not sure how uint64_t would be cast on 32 bit systems.

[dunglas]

After thinking about this a bit more, maybe the best option is C.uintptr_t. As Go's int type, it will have a different size depending on the platform capabilities, and it will prevent a conversion C-side.

[AlliBalliBaba]

Should it then be a map of C.uintptr_t? A slice still works, but it somehow feels weird to do this: phpThreads[C.uintptr_t(i)]

[dunglas]

it shouldn't be an issue

[dunglas]

By the way, I would like to contact you privately @AlliBalliBaba, I couldn't find a way to contact you, would you mind sending a mail or a DM on any social network?

[dunglas]

@AlliBalliBaba I simplified the code, rebased and renamed a function to match our CS in 756675f (#1073). Let's merge if it works for you.

[AlliBalliBaba]

Sounds good to me 👍

[dunglas]

Thank you @AlliBalliBaba!!

[AlliBalliBaba]

@dunglas fyi I sent you mail on [email protected]