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

Implement EnqueueExtensions #55

Closed codefromthecrypt closed 10 months ago

codefromthecrypt commented 10 months ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

This implements framework.EnqueueExtensions which is needed for common sample plugins, such as nodenumber

This also reduces namespace conflicts by moving the guest code that is related to protos into its own package. Notably there's a conflict on the framework symbol Node and the proto type.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

What are the benchmark results of this change?

goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e
                                                        │ before.txt  │              now.txt               │
                                                        │   sec/op    │   sec/op     vs base               │
PluginPreFilter/noop-wat/params:_small-12                 131.6n ± 0%   132.0n ± 1%   +0.30% (p=0.011 n=6)
PluginPreFilter/noop-wat/params:_real-12                  132.0n ± 0%   133.0n ± 1%   +0.76% (p=0.006 n=6)
PluginPreFilter/noop/params:_small-12                     277.1n ± 0%   289.1n ± 4%   +4.33% (p=0.002 n=6)
PluginPreFilter/noop/params:_real-12                      279.5n ± 4%   280.9n ± 2%        ~ (p=0.485 n=6)
PluginPreFilter/test/params:_small-12                     4.329µ ± 1%   4.342µ ± 1%        ~ (p=0.145 n=6)
PluginPreFilter/test/params:_real-12                      48.75µ ± 2%   47.95µ ± 1%   -1.64% (p=0.015 n=6)
PluginFilter/noop-wat/params:_small-12                    267.5n ± 1%   351.8n ± 1%  +31.50% (p=0.002 n=6)
PluginFilter/noop-wat/params:_real-12                     266.6n ± 0%   351.2n ± 1%  +31.76% (p=0.002 n=6)
PluginFilter/noop/params:_small-12                        421.3n ± 0%   544.9n ± 1%  +29.35% (p=0.002 n=6)
PluginFilter/noop/params:_real-12                         421.9n ± 1%   546.5n ± 1%  +29.52% (p=0.002 n=6)
PluginFilter/test/params:_small-12                        7.833µ ± 0%   7.952µ ± 0%   +1.52% (p=0.002 n=6)
PluginFilter/test/params:_real-12                         111.8µ ± 1%   111.4µ ± 1%        ~ (p=0.394 n=6)
PluginScore/noop-wat/params:_small-12                     268.6n ± 2%   351.5n ± 0%  +30.85% (p=0.002 n=6)
PluginScore/noop-wat/params:_real-12                      269.3n ± 1%   352.1n ± 1%  +30.76% (p=0.002 n=6)
PluginScore/noop/params:_small-12                         517.4n ± 1%   637.7n ± 0%  +23.24% (p=0.002 n=6)
PluginScore/noop/params:_real-12                          463.3n ± 0%   581.5n ± 0%  +25.50% (p=0.002 n=6)
PluginScore/test/params:_small-12                         4.695µ ± 0%   4.802µ ± 0%   +2.28% (p=0.002 n=6)
PluginScore/test/params:_real-12                          49.18µ ± 2%   48.73µ ± 1%        ~ (p=0.065 n=6)
PluginPrefilterFilterAndScore/noop-wat/params:_small-12   403.2n ± 1%   488.6n ± 0%  +21.18% (p=0.002 n=6)
PluginPrefilterFilterAndScore/noop-wat/params:_real-12    402.1n ± 1%   490.0n ± 1%  +21.88% (p=0.002 n=6)
PluginPrefilterFilterAndScore/noop/params:_small-12       655.9n ± 0%   785.6n ± 1%  +19.78% (p=0.002 n=6)
PluginPrefilterFilterAndScore/noop/params:_real-12        659.8n ± 1%   792.4n ± 0%  +20.09% (p=0.002 n=6)
PluginPrefilterFilterAndScore/test/params:_small-12       8.612µ ± 1%   9.027µ ± 1%   +4.81% (p=0.002 n=6)
PluginPrefilterFilterAndScore/test/params:_real-12        111.8µ ± 2%   112.8µ ± 1%        ~ (p=0.132 n=6)
PluginEnqueue/noop-wat-12                                               77.31n ± 1%
PluginEnqueue/noop-12                                                   112.7n ± 2%
PluginEnqueue/test-12                                                   130.7n ± 2%
geomean                                                   1.325µ        1.113µ       +12.93%
k8s-ci-robot commented 10 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codefromthecrypt

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)~~ [codefromthecrypt] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
codefromthecrypt commented 10 months ago

ok I cleaned up the PR so that it is ready to merge if ok (backfilled test coverage and notes).

After this, we can add guest config (e.g. plugin to read yaml or otherwise) and PreScore, so we can implement a realistic, configurable plugin like nodenumber.

codefromthecrypt commented 10 months ago

ok all good. bench should have become a little worse on noop, due to extra hook. Things look correct now.

codefromthecrypt commented 10 months ago

@sanposhiho PTAL I think I got everything (pre-squashed)

k8s-ci-robot commented 10 months ago

New changes are detected. LGTM label has been removed.

codefromthecrypt commented 10 months ago

thanks for the catch @sanposhiho!

codefromthecrypt commented 10 months ago

/unhold

codefromthecrypt commented 10 months ago

tomorrow I'll work on pre-score and hopefully config. after that, we should be able to port a basic plugin.