Skip to content

Conversation

ShadowCurse
Copy link
Contributor

Changes

Some benchmarks are very small and fast and this causes them to be unstable. We still keep an eye on the performance with longer benches.

Reason

Lower down variability in bench results.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this
    PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet
    contribution quality standards.

  • This functionality cannot be added in rust-vmm.

@ShadowCurse ShadowCurse self-assigned this Sep 17, 2024
@ShadowCurse ShadowCurse added Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Fix Indicates a fix to existing code Type: Performance labels Sep 17, 2024
@ShadowCurse ShadowCurse force-pushed the remove_too_fast_benches branch from 9289505 to 8c972ef Compare September 17, 2024 12:12
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.32%. Comparing base (a4b3d93) to head (56b8824).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4810   +/-   ##
=======================================
  Coverage   84.32%   84.32%           
=======================================
  Files         249      249           
  Lines       27512    27512           
=======================================
  Hits        23200    23200           
  Misses       4312     4312           
Flag Coverage Δ
5.10-c5n.metal 84.54% <ø> (ø)
5.10-m5n.metal 84.53% <ø> (-0.01%) ⬇️
5.10-m6a.metal 83.82% <ø> (-0.01%) ⬇️
5.10-m6g.metal 80.90% <ø> (ø)
5.10-m6i.metal 84.52% <ø> (-0.02%) ⬇️
5.10-m7g.metal 80.90% <ø> (ø)
6.1-c5n.metal 84.54% <ø> (ø)
6.1-m5n.metal 84.53% <ø> (-0.01%) ⬇️
6.1-m6a.metal 83.82% <ø> (-0.01%) ⬇️
6.1-m6g.metal 80.90% <ø> (ø)
6.1-m6i.metal 84.52% <ø> (ø)
6.1-m7g.metal 80.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Some benchmarks are very small and fast and this causes them to
be unstable. We can still keep an eye on the performance with
longer benches. Also increased number of runs and threshold.
This should be fine as all tests still run in nanoseconds.

Signed-off-by: Egor Lazarchuk <[email protected]>
@ShadowCurse ShadowCurse force-pushed the remove_too_fast_benches branch from 8c972ef to 0f04f16 Compare September 17, 2024 12:51
@ShadowCurse ShadowCurse requested a review from pb8o September 17, 2024 13:36
@ShadowCurse ShadowCurse requested review from pb8o and roypat September 17, 2024 16:17
@ShadowCurse ShadowCurse merged commit 5d762a8 into firecracker-microvm:main Sep 18, 2024
7 checks passed
@ShadowCurse ShadowCurse deleted the remove_too_fast_benches branch September 18, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed Type: Fix Indicates a fix to existing code Type: Performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants