Skip to content

RPS Workaround#48576

Merged
2 commits merged intodotnet:release/dev16.9from
333fred:provider-enabled
Oct 14, 2020
Merged

RPS Workaround#48576
2 commits merged intodotnet:release/dev16.9from
333fred:provider-enabled

Conversation

@333fred
Copy link
Member

@333fred 333fred commented Oct 13, 2020

Workaround for RPS.

@333fred 333fred requested a review from sharwell October 13, 2020 23:40
@333fred 333fred requested a review from a team as a code owner October 13, 2020 23:40
@Dotnet-GitSync-Bot
Copy link
Collaborator

Dotnet-GitSync-Bot commented Oct 13, 2020

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.

@333fred 333fred requested a review from a team October 13, 2020 23:42
Cosifne
Cosifne reviewed Oct 13, 2020
View reviewed changes
src/Workspaces/Remote/Core/ServiceHubRemoteHostClient.cs
return client;
}

static bool IsRpsMachine()
Copy link
Member

@Cosifne Cosifne Oct 13, 2020

Choose a reason for hiding this comment

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

Looks like we are going to disable the OOP call and use the in proc client?

Copy link
Member

@Cosifne Cosifne Oct 13, 2020

Choose a reason for hiding this comment

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

I wonder do we need to create a bug to track the progress of this.

Copy link
Member Author

@333fred 333fred Oct 13, 2020

Choose a reason for hiding this comment

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

Yes, that is correct.

Copy link
Contributor

@sharwell sharwell Oct 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like we are going to disable the OOP call and use the in proc client?

No, we are disabling the ability of RPS to wait for Roslyn asynchronous operations to complete when those operations run in ServiceHub. In other words, RPS says "wait for Roslyn to complete work", and Roslyn says "not if you are RPS".

Cosifne
Cosifne approved these changes Oct 13, 2020
View reviewed changes
Copy link
Member

@Cosifne Cosifne left a comment

Choose a reason for hiding this comment

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

I assume we have already tested in the VS side that this change works

@333fred
Copy link
Member Author

333fred commented Oct 13, 2020

I assume we have already tested in the VS side that this change works

Yes

@ghost
Copy link

ghost commented Oct 13, 2020

Hello @333fred!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost
ghost approved these changes Oct 13, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit c757155 into dotnet:release/dev16.9 Oct 14, 2020
@333fred 333fred deleted the provider-enabled branch October 14, 2020 03:22
genlu added a commit to genlu/roslyn that referenced this pull request Oct 23, 2020
@genlu
Revert "Merge pull request dotnet#48576 from 333fred/provider-enabled"
c691269 
This reverts commit c757155, reversing
changes made to a8081c3.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sharwell sharwell Awaiting requested review from sharwell

2 more reviewers

@Cosifne Cosifne Cosifne approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Infrastructure auto-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@333fred @Dotnet-GitSync-Bot @sharwell @Cosifne