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

feat(bench): Re-enable cargo bench #612

Merged
merged 11 commits into from
Jun 20, 2024
Merged

Conversation

Mossaka
Copy link
Member

@Mossaka Mossaka commented Jun 6, 2024

This commit reverts back a change made by commit SHA 6882dd5 and fixed all the build errors for the benchmark.

  • Add cargo bench to GitHub Actions

Closes #611

@Mossaka Mossaka marked this pull request as ready for review June 6, 2024 22:03
@Mossaka Mossaka requested review from jsturtevant and jprendes June 6, 2024 22:04
@Mossaka Mossaka mentioned this pull request Jun 7, 2024
13 tasks
@Mossaka Mossaka closed this Jun 7, 2024
@Mossaka Mossaka reopened this Jun 7, 2024
@Mossaka
Copy link
Member Author

Mossaka commented Jun 7, 2024

The benchmarking in the CI finishes in 23 minutes. FWIW @jsturtevant

@Mossaka Mossaka requested a review from devigned June 8, 2024 21:16
@Mossaka Mossaka closed this Jun 8, 2024
@Mossaka Mossaka reopened this Jun 8, 2024
@Mossaka Mossaka force-pushed the renable-bench branch 2 times, most recently from fad7e37 to bae8997 Compare June 13, 2024 19:32
jprendes
jprendes previously approved these changes Jun 13, 2024
Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I'm going to take another look tomorrow if it hasn't been merged it yet.

Thanks for working on this!

.github/workflows/benchmarks.yml Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Mossaka added 7 commits June 13, 2024 21:47
This commit reverts back a change made by commit SHA 6882dd5 and fixed all the build errors for the benchmark.

Closes containerd#611

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
this commit adds a new github action that runs every Sunday to measure the benchmark

Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
jsturtevant
jsturtevant previously approved these changes Jun 13, 2024
Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

wasi.delete()?;
res
fn run_wasmtime_test_with_spec(wasmbytes: &[u8]) -> Result<u32, Error> {
let (exit_code, _, _) = WasiTest::<WasmtimeTestInstance>::builder()?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note that this test framework doesn't test starting containers in with containerd. One of the things we are interested in is the entire startup time including interactions with containerd and should make sure we have a test that covers that via a follow up

.github/workflows/benchmarks.yml Show resolved Hide resolved
Mossaka added 2 commits June 13, 2024 21:53
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
@Mossaka
Copy link
Member Author

Mossaka commented Jun 13, 2024

@jsturtevant It turns out that wasmedge benchmarking wasn't working - it times out after 10 seconds. I need more time to investigate the WasmEdge case, so for this PR, I removed it.

@Mossaka Mossaka requested review from jsturtevant and jprendes June 13, 2024 22:49
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
@Mossaka Mossaka requested a review from jprendes June 19, 2024 22:30
Copy link
Contributor

@devigned devigned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Mossaka
Copy link
Member Author

Mossaka commented Jun 20, 2024

Going to merge this in. Will work on the follow ups.

@Mossaka Mossaka merged commit 12fd000 into containerd:main Jun 20, 2024
52 checks passed
@Mossaka Mossaka deleted the renable-bench branch June 20, 2024 18:51
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.

All bench tests are "Ignored"
4 participants