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.

Sign up for GitHub

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

Jump to bottom

Remove dependency on PerfCounter.dll #45142

Merged
merged 3 commits into from
Dec 1, 2020
Merged

Remove dependency on PerfCounter.dll #45142

merged 3 commits into from
Dec 1, 2020

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Nov 24, 2020

This dependency was unnecessary and only for calling through to pdh.dll.

PerfCounter.dll is still needed for the consumption side of performance
counters, but removing from the dependency from the .NET can help enable
some scenarios like reading non-.NET counters.

@ericstj
Remove dependency on PerfCounter.dll
f589278 
This dependency was unnecessary and only for calling through to pdh.dll.

PerfCounter.dll is still needed for the consumption side of performance
counters, but removing from the dependency from the .NET can help enable
some scenarios like reading non-.NET counters.
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Nov 24, 2020

Tagging subscribers to this area: @tommcdon, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

This dependency was unnecessary and only for calling through to pdh.dll.

PerfCounter.dll is still needed for the consumption side of performance
counters, but removing from the dependency from the .NET can help enable
some scenarios like reading non-.NET counters.

Author: ericstj
Assignees: -
Labels:

area-System.Diagnostics
Milestone: -

@ericstj
Copy link
Member Author

ericstj commented Nov 24, 2020

Contributes to #41144

@ericstj
Copy link
Member Author

ericstj commented Nov 24, 2020 •
edited
Loading

So running on ARM we now have:

=== TEST EXECUTION SUMMARY ===
   System.Diagnostics.PerformanceCounter.Tests  Total: 92, Errors: 0, Failed: 9, Skipped: 1, Time: 122.870s

https://dev.azure.com/dnceng/public/_build/results?buildId=897861&view=ms.vss-test-web.build-test-results-tab&runId=28687154&paneView=debug

The tests which fail are failing due to inability to read .NET perf counters. This is due to absense of netfxperf.dll/perfcounter.dll. I'm going through and fixing all the test conditions to address this.

@ericstj
Copy link
Member Author

ericstj commented Nov 25, 2020

Fixes #34409
Fixes #24176

@ericstj ericstj requested a review from Anipik November 30, 2020 23:26
@ericstj
Copy link
Member Author

ericstj commented Dec 1, 2020

@Anipik @danmosemsft can you have a look at this one? It enables the reading of non-.NET perf counters on Windows ARM64.

danmoseley
danmoseley approved these changes Dec 1, 2020
View reviewed changes
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

LGTM - I see FormatFromRawValue is just a pass through in perfcounter.dll

@danmoseley danmoseley merged commit 808f543 into dotnet:master Dec 1, 2020
Anipik
Anipik reviewed Dec 2, 2020
View reviewed changes
Copy link
Contributor

@Anipik Anipik left a comment

Choose a reason for hiding this comment

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

lgtm

@ericstj ericstj mentioned this pull request Dec 2, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Anipik Anipik Anipik left review comments

@danmoseley danmoseley danmoseley approved these changes

Assignees
No one assigned
Labels
area-System.Diagnostics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ericstj @Dotnet-GitSync-Bot @danmoseley @Anipik