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

Remove barriers and spinlock in epoch_enter and epoch_exit #2796

Merged
merged 7 commits into from
Sep 14, 2023

Conversation

Alan-Jowett
Copy link
Member

@Alan-Jowett Alan-Jowett commented Aug 28, 2023

Description

Switch epoch logic from relying on spin-lock for protecting to relying on running at IRQL == DISPATCH_LEVEL.

Batching == 1

Before:

C:\bpf_perf>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -b 1 -c  50000000
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,119,119,115,117,116,118

After:

C:\bpf_perf>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -b 1 -c  50000000
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,31,30,31,31,31,33

Batching == 64

Before:

C:\bpf_perf>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -c  50000000
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,10,11,10,9,10,10

After:

C:\bpf_perf>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -c  50000000
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,8,7,7,8,8,8

Testing

CI/CD + performance + stress.

Documentation

Yes.

Installation

No.

Resolves: #2787

@Alan-Jowett
Copy link
Member Author

After generating SPGO data and applying it:

Old epoch:

C:\test>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -b 1
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,107,109,108,109,111,113

C:\test>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline"
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,7,6,6,7,6,6

New epoch:

C:\test>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline" -b 1
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,31,28,29,30,30,25

C:\test>RelWithDebInfo\bpf_performance_runner.exe -i tests.yml -e .sys -t "Baseline"
Test,CPU 0,CPU 1,CPU 2,CPU 3,CPU 4,CPU 5
Baseline,4,5,5,5,5,5

dthaler
dthaler previously approved these changes Sep 1, 2023
Copy link
Collaborator

@dthaler dthaler left a comment

Choose a reason for hiding this comment

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

Ok but see suggestions.

docs/EpochBasedMemoryManagement.md Outdated Show resolved Hide resolved
docs/EpochBasedMemoryManagement.md Outdated Show resolved Hide resolved
docs/EpochBasedMemoryManagement.md Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/platform/ebpf_epoch.c Outdated Show resolved Hide resolved
@Alan-Jowett
Copy link
Member Author

Blocked on: #2783 as it has the updated version of usersim with kevent.

@Alan-Jowett Alan-Jowett force-pushed the speed_up_epoch branch 2 times, most recently from 6fc07d6 to b5b45f1 Compare September 5, 2023 22:42
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
Alan Jowett and others added 5 commits September 6, 2023 08:46
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
Co-authored-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
libs/runtime/ebpf_epoch.c Outdated Show resolved Hide resolved
Alan-Jowett and others added 2 commits September 6, 2023 12:59
Co-authored-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #2796 (bddd81e) into main (ebc4816) will decrease coverage by 0.22%.
Report is 2 commits behind head on main.
The diff coverage is 95.39%.

@@            Coverage Diff             @@
##             main    #2796      +/-   ##
==========================================
- Coverage   86.71%   86.50%   -0.22%     
==========================================
  Files         140      140              
  Lines       25894    25926      +32     
==========================================
- Hits        22454    22426      -28     
- Misses       3440     3500      +60     
Files Changed Coverage Δ
libs/execution_context/ebpf_core.c 91.51% <75.00%> (-0.07%) ⬇️
libs/execution_context/ebpf_program.c 87.28% <75.00%> (+0.21%) ⬆️
libs/runtime/ebpf_epoch.c 95.94% <97.22%> (+<0.01%) ⬆️
libs/execution_context/ebpf_link.c 91.14% <100.00%> (ø)
libs/runtime/unit/platform_unit_test.cpp 99.69% <100.00%> (+<0.01%) ⬆️

... and 6 files with indirect coverage changes

Copy link
Collaborator

@shankarseal shankarseal left a comment

Choose a reason for hiding this comment

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

My concern with this changes is that this may frequently queue DPCs to cores thereby stalling other DPCs generated by interrupts such as arriving network packets. It will be necessary to test these changes using XDP tests with high bandwidth of inbound traffic and also with other multithreaded stress scenarios to ensure there are no regressions.

an alternative was discussed here and eventully rejected. #2844

@Alan-Jowett Alan-Jowett added this pull request to the merge queue Sep 14, 2023
Merged via the queue into microsoft:main with commit 4c3d2cd Sep 14, 2023
69 checks passed
@Alan-Jowett Alan-Jowett deleted the speed_up_epoch branch September 14, 2023 23:02
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.

Reduce epoch_enter/exit overhead
3 participants