Skip to content

Fix CPU is consumed by polling frequently when there is no subscriber#4735

Merged
cvpoienaru merged 12 commits intomicrosoft:mainfrom
nohwnd:Fix-tight-loop-when-waiting
Nov 14, 2023
Merged

Fix CPU is consumed by polling frequently when there is no subscriber#4735
cvpoienaru merged 12 commits intomicrosoft:mainfrom
nohwnd:Fix-tight-loop-when-waiting

Conversation

@nohwnd
Copy link
Member

@nohwnd nohwnd commented Nov 3, 2023 •
edited
Loading

Description

We disabled pre-started testhosts in 17.6.0 because the waiting host could consume large amounts of CPU when data were waiting on the pipeline and there were no consumers on vstest.console side. This is easy to reproduce when you set VSTEST_HOSTPRESTART_COUNT = 1 or more, and enable diagnostic logs.

What happens is that we run init on the pre-started testhost and after we remove the CheckVersionWithHost subscriber, the diag log will push TestMesage: Logging TestHost Diagnostics in file... which will be wating on the wire and causing tight loop of notifications to subscriber that is not there.

In logs this shows up like this, with 0ms since the last poll, because the Poll returns immediately, and the notification as well, causing this tight loop:

TpTrace Verbose: 0 : 59440, 12, 2023/11/03, 12:18:50.769, 655225963354, vstest.console.exe, TcpClientExtensions.MessageLoopAsync: NotifyDataAvailable remoteEndPoint: 127.0.0.1:59895 localEndPoint: 127.0.0.1:59892
TpTrace Verbose: 0 : 59440, 12, 2023/11/03, 12:18:50.769, 655225963520, vstest.console.exe, TcpClientExtensions.MessageLoopAsync: Polling on remoteEndPoint: 127.0.0.1:59895 localEndPoint: 127.0.0.1:59892 after 0 ms

Instead of this we add a flag that will wait for the maximum of 1 second for a subscriber to subscribe to data reading, and if that does not happen we repeat the loop.

In log it shows like this:

TpTrace Verbose: 0 : 59440, 12, 2023/11/03, 12:18:50.769, 655225963354, vstest.console.exe, TcpClientExtensions.MessageLoopAsync: NotifyDataAvailable remoteEndPoint: 127.0.0.1:59895 localEndPoint: 127.0.0.1:59892
TpTrace Verbose: 0 : 59440, 12, 2023/11/03, 12:18:51.769, 655225963520, vstest.console.exe, TcpClientExtensions.MessageLoopAsync: Polling on remoteEndPoint: 127.0.0.1:59895 localEndPoint: 127.0.0.1:59892 after 1000 ms

Where we correctly wait for the subscriber to subscribe. We also return immediately when we are being cancelled.

There are probably other causes for this to happen, but this should solve all of them.

Related issue

Fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1896342
Fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1835463

nohwnd
nohwnd commented Nov 3, 2023
View reviewed changes
@nohwnd
Copy link
Member Author

nohwnd commented Nov 3, 2023

@cvpoienaru Please fix the tests, and re-enable the PreStartCount to 2. 🙏🙇

nohwnd
nohwnd commented Nov 3, 2023
View reviewed changes
src/Microsoft.TestPlatform.CommunicationUtilities/LengthPrefixCommunicationChannel.cs
MessageReceived.SafeInvoke(this, new MessageReceivedEventArgs { Data = data }, "LengthPrefixCommunicationChannel: MessageReceived");
}
else
{
Copy link
Member Author

@nohwnd nohwnd Nov 3, 2023

Choose a reason for hiding this comment

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

This message is unnecessary, and way too much detail. with prestart it is normal that client is waiting for his turn and this would just add yet another message to the log that would always repeat.

@nohwnd nohwnd changed the title Fix cpu usage by polling frequently when there is no subscriber Fix CPU is consumed by polling frequently when there is no subscriber Nov 3, 2023
@nohwnd
Copy link
Member Author

nohwnd commented Nov 7, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 7, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd nohwnd mentioned this pull request Nov 9, 2023
Closed
@cvpoienaru cvpoienaru enabled auto-merge (squash) November 14, 2023 14:30
@cvpoienaru cvpoienaru merged commit 5d3d844 into microsoft:main Nov 14, 2023
ThorstenSauter referenced this pull request in ThorstenSauter/NoPlan Feb 6, 2024
@renovate
chore(deps): update non-major nuget dependencies (#623)
dbe24e2 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[Microsoft.Identity.Web](https://togithub.com/AzureAD/microsoft-identity-web)
| `2.16.1` -> `2.17.0` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.Identity.Web/2.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.Identity.Web/2.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.Identity.Web/2.16.1/2.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.Identity.Web/2.16.1/2.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [Microsoft.NET.Test.Sdk](https://togithub.com/microsoft/vstest) |
`17.8.0` -> `17.9.0` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.NET.Test.Sdk/17.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.NET.Test.Sdk/17.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.NET.Test.Sdk/17.8.0/17.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.NET.Test.Sdk/17.8.0/17.9.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>AzureAD/microsoft-identity-web
(Microsoft.Identity.Web)</summary>

###
[`v2.17.0`](https://togithub.com/AzureAD/microsoft-identity-web/blob/HEAD/changelog.md#2170)

[Compare
Source](https://togithub.com/AzureAD/microsoft-identity-web/compare/2.16.1...2.17.0)

\=========

-   Updated to Microsoft.IdentityModel.\* 7.3.1 and MSAL.NET 4.59.0

##### New features

- Added support for Microsoft.NET.Sdk.Worker. See [Worker calling
APIs](https://togithub.com/AzureAD/microsoft-identity-web/wiki/worker%E2%80%90app%E2%80%90calling%E2%80%90downstream%E2%80%90apis)
- Added support for Managed identity when calling a downstream API on
behalf of the app. See [Calling APIs with Managed
Identity](https://togithub.com/AzureAD/microsoft-identity-web/wiki/calling-apis-with-managed-identity)
and [PR
2650](https://togithub.com/AzureAD/microsoft-identity-web/pull/2650).
For details see [PR
#&#8203;2645](https://togithub.com/AzureAD/microsoft-identity-web/issues/2645)

##### Bug fixes

- In OWIN applications, GetTokenForUserAsync now respects the
ClaimsPrincipal. See issue
[#&#8203;2629](https://togithub.com/AzureAD/microsoft-identity-web/issues/2629)
for details.
- After setting `AddTokenAcquisition(useSingleton:true)` to use token
acquisition as a singleton, if you use `.AddMicrosoftGraph` and/or
`.AddDownstreamApi` after this call,
the GraphServiceClient and IDownstreamApis are now registered as a
singleton service. For details see [PR
#&#8203;2645](https://togithub.com/AzureAD/microsoft-identity-web/issues/2645)
- Added check Against Injection Attacks. For details see [PR
2619](https://togithub.com/AzureAD/microsoft-identity-web/issues/2619)

##### Engineering excellence

- Added a benchmark running on PR merges, available from
<https://azuread.github.io/microsoft-identity-web/benchmarks> on GitHub
pages

</details>

<details>
<summary>microsoft/vstest (Microsoft.NET.Test.Sdk)</summary>

###
[`v17.9.0`](https://togithub.com/microsoft/vstest/releases/tag/v17.9.0)

##### What's Changed

- Remove dependency on Nuget.Frameworks by
[@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4693](https://togithub.com/microsoft/vstest/pull/4693)
- Add platform guard by
[@&#8203;dotMorten](https://togithub.com/dotMorten) in
[https://github.com/microsoft/vstest/pull/4737](https://togithub.com/microsoft/vstest/pull/4737)
- Remove javascript in `HtmlLogger` by
[@&#8203;daveMueller](https://togithub.com/daveMueller) in
[https://github.com/microsoft/vstest/pull/4744](https://togithub.com/microsoft/vstest/pull/4744)
- Fix CPU is consumed by polling frequently when there is no subscriber
by [@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4735](https://togithub.com/microsoft/vstest/pull/4735)
- Create a proper MSBuild ToolTask based VSTestTask by
[@&#8203;mcartoixa](https://togithub.com/mcartoixa) in
[https://github.com/microsoft/vstest/pull/2702](https://togithub.com/microsoft/vstest/pull/2702)
- Update SDK, copy logger by
[@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4755](https://togithub.com/microsoft/vstest/pull/4755)
- Fix staircase in logger by
[@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4756](https://togithub.com/microsoft/vstest/pull/4756)
- Update dump client to latest by
[@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4724](https://togithub.com/microsoft/vstest/pull/4724)
- \[rel/17.9] Disable testhost prestart by
[@&#8203;nohwnd](https://togithub.com/nohwnd) in
[https://github.com/microsoft/vstest/pull/4836](https://togithub.com/microsoft/vstest/pull/4836)

##### New Contributors

- [@&#8203;jithu7432](https://togithub.com/jithu7432) made their first
contribution in
[https://github.com/microsoft/vstest/pull/4698](https://togithub.com/microsoft/vstest/pull/4698)
- [@&#8203;mcartoixa](https://togithub.com/mcartoixa) made their first
contribution in
[https://github.com/microsoft/vstest/pull/2702](https://togithub.com/microsoft/vstest/pull/2702)

**Full Changelog**:
microsoft/vstest@v17.8.0...v17.9.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am" (UTC), Automerge - At any
time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ThorstenSauter/NoPlan).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This was referenced Mar 17, 2026
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Merged
Open
Open
Open
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cvpoienaru cvpoienaru cvpoienaru approved these changes

@MarcoRossignoli MarcoRossignoli MarcoRossignoli approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nohwnd @cvpoienaru @MarcoRossignoli