kubernetes-sigs / kube-scheduler-wasm-extension

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

implements ScorePlugin #35

Closed codefromthecrypt closed 11 months ago

codefromthecrypt commented 11 months ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

This implements ScorePlugin. Notably, this reduces the size of the score returnable by wasm to int32 from int64 for reasons elaborated in the RATIONALE.md.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Thanks a lot to @anuraaga and @ncruces for helping with the wasm aspects of the rationale.

Thanks a lot to @kerthcet and @sanposhiho for helping with the value space aspects of the score plugin.

Does this PR introduce a user-facing change?

Only notable behavior is reducing the value range of score produced by wasm plugins to max int32.

What are the benchmark results of this change?

goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e
                                            │   old.txt   │               new.txt               │
                                            │   sec/op    │   sec/op     vs base                │
PluginFilter/noop/params:_small-12            179.5n ± 0%   181.0n ± 0%  +0.84% (p=0.002 n=6)
PluginFilter/noop/params:_real-12             178.8n ± 1%   182.3n ± 1%  +1.96% (p=0.004 n=6)
PluginFilter/filter-simple/params:_small-12   6.750µ ± 2%
PluginFilter/filter-simple/params:_real-12    50.62µ ± 2%
PluginFilter/noop-wat/params:_small-12                      120.1n ± 0%
PluginFilter/noop-wat/params:_real-12                       119.8n ± 0%
PluginFilter/wasm/params:_small-12                          6.671µ ± 1%
PluginFilter/wasm/params:_real-12                           50.26µ ± 2%
PluginScore/noop-wat/params:_small-12                       120.5n ± 2%
PluginScore/noop-wat/params:_real-12                        122.0n ± 1%
PluginScore/noop/params:_small-12                           244.5n ± 4%
PluginScore/noop/params:_real-12                            206.5n ± 1%
PluginScore/wasm/params:_small-12                           5.082µ ± 5%
PluginScore/wasm/params:_real-12                            49.80µ ± 1%
geomean                                       1.820µ        746.4n       +1.39%               ¹
¹ benchmark set differs from baseline; geomeans may not be comparable
kerthcet commented 11 months ago

/hold

codefromthecrypt commented 11 months ago

In theory, yes, we have no idea how people design their plugins. Although after normalizing, they will be in the range of [0, 100]

ok I will put the note in that normalizing means [0, 100] so basically regardless of number above 100 it normalizes to 100.

codefromthecrypt commented 11 months ago

So, next steps for me are:

Will push a commit to include this!

codefromthecrypt commented 11 months ago

ok spent another hour plus on docs. I think that's way over budget for me and unparalleled in practice.

Next is test.

codefromthecrypt commented 11 months ago

ok addressed all the TODO I'm aware of, PTAL!

codefromthecrypt commented 11 months ago

once last feedback is in, I will squash so that the commit history isn't junked up

codefromthecrypt commented 11 months ago

thanks for the looks folks!

k8s-ci-robot commented 11 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anuraaga, codefromthecrypt, kerthcet, sanposhiho

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/OWNERS)~~ [kerthcet,sanposhiho] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment