kubernetes-sigs / kube-scheduler-wasm-extension

All the things to make the scheduler extendable with wasm.
Apache License 2.0
95 stars 18 forks source link

better throughput without GC? #109

Closed sanposhiho closed 1 month ago

sanposhiho commented 2 months ago

We're using a great nottinygc to reduce the cost of GC in wasm guests.

Swapping this added 110KB to the base size of the wasm guest, but resulted in 48pct drop in execution time of a simple plugin, in a scale of hundreds of microseconds. https://github.com/codefromthecrypt/kube-scheduler-wasm-extension/blob/main/guest/RATIONALE.md#why-do-we-recommend-nottinygc-but-not-import-it-by-default

But, nottinygc is archived.

I've had an another idea for this challange coming from GC actually since before.

When api.Module is closed, I suppose all resources allocated to the module are released. And, then I'm wondering if it'd be possible to get a better performance by stopping GC on wasm module from tinygo with -gc=leaking flag, and instead create/delete modules every time on host side. The performance depends on how expensive creating a new guest is though.

This issue intends to research that possibility, with seeking concerns around it too.

sanposhiho commented 1 month ago

/priority next-release

sanposhiho commented 1 month ago

I give it a try. So, in short, -gc=leaking + module recreation increases the throughput, at least with my implementation.

I tried it at no-gc branch,

And, run some benchmarks.

Before:

$ make bench-example
goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e/scheduler
BenchmarkExample_NodeNumber/OnlyFilter_Advanced/Run-12           1687575               741.3 ns/op
BenchmarkExample_NodeNumber/OnlyFilter_Advanced_(with_normal_gc)/Run-12           490719              2438 ns/op
BenchmarkExample_NodeNumber/Advanced/Run-12                                        33591             36176 ns/op
BenchmarkExample_NodeNumber/Advanced_(with_normal_gc)/Run-12                       10000            103483 ns/op

After:

$ make bench-example
goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e/scheduler
BenchmarkExample_NodeNumber/OnlyFilter_Advanced/Run-12             53358             18770 ns/op
BenchmarkExample_NodeNumber/OnlyFilter_Advanced_(with_normal_gc)/Run-12            54272             18816 ns/op
BenchmarkExample_NodeNumber/Advanced/Run-12                                        12394             95442 ns/op
BenchmarkExample_NodeNumber/Advanced_(with_normal_gc)/Run-12                        4820            225602 ns/op

The result in After shows using gc=leaking is faster than normal tinygo gc.

But, looking at the comparison between Before and After, Advanced_(with_normal_gc) takes more time in After than Before. We recreate modules in goroutine, so the recreation doesn't block the scheduling directly, but from this result, we can have a guess it does indirectly; a frequent recreation of modules causes the wazero runtime executing wasm functions slower somehow.

sanposhiho commented 1 month ago

I know there could be more things that I could look into to know where is the bottleneck and possibly find out the solution for them, but, at least, it's not a low-hanging fruit.

Go is about to support wasmexport and I'm guessing we're likely gonna move our SDK to Go base, from TinyGo base. https://github.com/golang/go/issues/65199

Then, all the effort here could be waste after all. So, it's much better to put effort for other necessary functionalities rather than this issue, while waiting for an arrival of Go's export support.

For now, I'm closing this issue and we can reopen if necessary, when we find TinyGo GC stuff optimization necessary again.

/close

k8s-ci-robot commented 1 month ago

@sanposhiho: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/issues/109#issuecomment-2272947109): >I know there could be more things that I could look into to know where is the bottleneck and possibly find out the solution for them, but, at least, it's not a low-hanging fruit. > >Go is about to support `wasmexport` and I'm _guessing_ we're likely gonna move our SDK to Go base, from TinyGo base. >https://github.com/golang/go/issues/65199 > >Then, all the effort here could be waste after all. For now, I'm closing this issue and we can reopen if necessary, when we find TinyGo GC stuff optimization necessary again. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.