-
Notifications
You must be signed in to change notification settings - Fork 240
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
Improve ringbuffer performance #4027
Draft
Alan-Jowett
wants to merge
2
commits into
microsoft:main
Choose a base branch
from
Alan-Jowett:rb_perf
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+266
−169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mtfriesen
reviewed
Nov 20, 2024
mtfriesen
reviewed
Nov 20, 2024
mtfriesen
reviewed
Nov 20, 2024
mtfriesen
reviewed
Nov 20, 2024
dthaler
reviewed
Nov 20, 2024
Alan-Jowett
requested review from
poornagmsft,
saxena-anurag,
shankarseal,
dv-msft,
gtrevi,
shpalani,
matthewige and
rectified95
as code owners
November 20, 2024 20:50
mikeagun
reviewed
Nov 22, 2024
There exists a race condition around the producer wrapping. |
Alan-Jowett
force-pushed
the
rb_perf
branch
2 times, most recently
from
November 23, 2024 22:56
d1c0713
to
a5fe86d
Compare
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request includes significant changes to the eBPF ring buffer implementation, focusing on improving synchronization, memory management, and adding new functionality. The most important changes include the introduction of spin locks for better concurrency control, a new flag-based system for record states, and the addition of a stress test for the ring buffer.
Synchronization and Memory Management Improvements:
libs/runtime/ebpf_ring_buffer.c
: Introducedcxplat_spin_lock_t
for both producer and consumer locks, replacing the previousebpf_lock_t
. This change improves concurrency control by using spin locks, which are more efficient in high-contention scenarios.libs/runtime/ebpf_ring_buffer.c
: Modified theebpf_ring_buffer_reserve
andebpf_ring_buffer_output
functions to use a new flag-based system for record states, ensuring better synchronization and memory management. [1] [2]New Flag-Based System for Record States:
libs/shared/ebpf_ring_buffer_record.h
: Introduced new flagsEBPF_RING_BUFFER_RECORD_FLAG_LOCKED
andEBPF_RING_BUFFER_RECORD_FLAG_DISCARDED
to manage the state of records in the ring buffer. This change simplifies the handling of record states and improves code readability. [1] [2]Additional Functionality:
libs/runtime/unit/platform_unit_test.cpp
: Added a new stress test casering_buffer_stress
to evaluate the performance and reliability of the ring buffer under high load conditions. This test helps ensure the robustness of the ring buffer implementation.Other Notable Changes:
libs/api/ebpf_api.cpp
: Added a loop to wait for the producer to finish writing if the record is locked, ensuring data consistency.external/usersim
: Updated the submodule commit reference to point to the latest commit.Testing
CI/CD + dedicated stress test
Documentation
No.
Installation
No.