Add resiliency to Resource Monitoring in Linux #6489
Conversation
This file contains hidden or 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
evgenyfedorov2
requested changes
Jun 5, 2025
| { | ||
| return []; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| List<DiskStats> diskStatsList = _diskStatsReader.ReadAll(); |
There was a problem hiding this comment.
I am sorry, I did not have time to review previous PRs, but here we have two List<> allocations - first inside the .ReadAll() method and second with .ToList() on line 160.
- Can we fold it into one allocation?
- Can we avoid allocations at all and use object pooling for the List<>?
- Can we do
.ToArray()on line 160 instead of.ToList()because we don't really need it to be List<> further in the flow?
There was a problem hiding this comment.
If you feel like it is not in scope of this PR, please address in a follow-up PR, of course
There was a problem hiding this comment.
I moved filtering to ReadAll(), so filtering happens on read, in result we don't have to enumerate twice, and additionally we skip second allocation. Also, switched to array as suggested for performance reasons
...ies/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/Disk/LinuxSystemDiskMetrics.cs
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/Disk/LinuxSystemDiskMetrics.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/LinuxNetworkMetricsTests.cs
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/Network/LinuxNetworkMetrics.cs
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/Network/LinuxNetworkMetrics.cs
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/Network/LinuxNetworkMetrics.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/LinuxNetworkMetricsTests.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/LinuxNetworkMetricsTests.cs
Outdated
Show resolved
Hide resolved
dotnet-policy-service
bot
added
waiting-author-feedback 📭
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.
Partially fixes #6478
This PR address Linux RM for Disk and Networking. To make PR smaller CPU and Memory will be addressed in separated ones. Later I will check what we can do for Windows.
Microsoft Reviewers: Open in CodeFlow