test(vsock): add latency test#5603
test(vsock): add latency test#5603aaron-ang wants to merge 3 commits intofirecracker-microvm:mainfrom
Conversation
aaron-ang
force-pushed
the
vsock-latency-perf
branch
from
90ff871 to
75e4ea8
Compare
|
hi @raduiliescu @Manciukic could you please take a look? |
|
Hey @aaron-ang, thanks for the contribution! I only had a very quick look, but I was wondering whether we need both |
JamesC1305
added
the
Status: Awaiting author
|
Hi @aaron-ang, Just wanted to see if you had a chance to read our previous update? Let us know if you need anything more from us. Thank you! |
|
Hi @JamesC1305, apologies as I have been really busy the past few weeks. I will look into this within this week. |
aaron-ang
force-pushed
the
vsock-latency-perf
branch
from
75e4ea8 to
1a80bd9
Compare
|
My setup: I did some stats aggregation (ping-uds has slightly higher overall latency):
I think both should be kept. |
Manciukic
left a comment
Manciukic
left a comment
There was a problem hiding this comment.
Overall LGTM, just a nit to reduce boilerplate code. I need to double check this works, I will kickoff our performance CI as well so we can run these tests
|
|
||
| @pytest.mark.nonci | ||
| @pytest.mark.parametrize("vcpus", [1, 2], ids=["1vcpu", "2vcpu"]) | ||
| def test_vsock_latency_g2h(uvm_plain_acpi, vcpus, metrics, bin_vsock_path): |
There was a problem hiding this comment.
nit: we can avoid some duped code by having the test setup in a separate fixture for all the tests in the file
@pytest.fixture
def vsock_uvm(uvm_plain_acpi, request):
"""Fixture to initialize a microVM with vsock device."""
vcpus = request.param if hasattr(request, "param") else 1
mem_size_mib = 1024
vm = uvm_plain_acpi
vm.spawn(log_level="Info", emit_metrics=True)
vm.basic_config(vcpu_count=vcpus, mem_size_mib=mem_size_mib)
vm.add_net_iface()
vm.api.vsock.put(vsock_id="vsock0", guest_cid=3, uds_path="/" + VSOCK_UDS_PATH)
vm.start()
vm.pin_threads(0)
return vm
then it can be used as
@pytest.mark.parametrize("vsock_uvm", [1, 2], indirect=True, ids=["1vcpu", "2vcpu"])
There was a problem hiding this comment.
@aaron-ang can you use the fixture here as well?
|
Perf test dry-run: https://buildkite.com/firecracker/performance-a-b-tests/builds/833 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5603 +/- ##
=======================================
Coverage 83.02% 83.02%
=======================================
Files 276 276
Lines 29428 29428
=======================================
Hits 24433 24433
Misses 4995 4995
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| """ | ||
| rounds = 15 | ||
| requests_per_round = 30 | ||
| delay_sec = 0.01 |
There was a problem hiding this comment.
I was looking at the results and there's quite some big spread. Maybe removing the delay like we did in the network test may help? Did you try with delay_sec = 0?
|
Plotting the results, it's pretty slow compared to what we would have expected. Surprisingly some instances are much much faster. I think removing the delay would make this better.
|
cool, thanks! I will incorporate your feedback this week |
Create simple ping latency test in vsock_helper host tools Signed-off-by: Aaron Ang <aaron.angyd@gmail.com>
Signed-off-by: Aaron Ang <aaron.angyd@gmail.com>
aaron-ang
force-pushed
the
vsock-latency-perf
branch
from
e7912cd to
dd13239
Compare
|
@Manciukic please review |
Signed-off-by: Aaron Ang <aaron.angyd@gmail.com>
aaron-ang
force-pushed
the
vsock-latency-perf
branch
from
dd13239 to
aa6f57b
Compare
Manciukic
left a comment
Manciukic
left a comment
There was a problem hiding this comment.
Just one nit but I haven't looked too deeply. I'll give it another go in our perf pipeline
|
|
||
| @pytest.mark.nonci | ||
| @pytest.mark.parametrize("vcpus", [1, 2], ids=["1vcpu", "2vcpu"]) | ||
| def test_vsock_latency_g2h(uvm_plain_acpi, vcpus, metrics, bin_vsock_path): |
There was a problem hiding this comment.
@aaron-ang can you use the fixture here as well?
|
Perf test dry run : https://buildkite.com/firecracker/performance-a-b-tests/builds/866/ |
|
It looks better now! Not as good as network, but that's a separate problem (unit is microseconds) |
| assert response == b"", ( | ||
| f"Connection not closed: response received '{response.decode('utf-8')}'" | ||
| ) |
There was a problem hiding this comment.
style check suggests
| assert response == b"", ( | |
| f"Connection not closed: response received '{response.decode('utf-8')}'" | |
| ) | |
| assert ( | |
| response == b"" | |
| ), f"Connection not closed: response received '{response.decode('utf-8')}'" |
please run ./tools/devtool fmt
3 participants
Create simple ping latency test in vsock_helper host tools
Changes
Close #2353.
Reason
vsock_helper.cwithpingandping-udscommands that emit per‑request RTTstest_vsock.pythat exercise guest -> host and host -> guest paths.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
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.