Skip to content

Revert "BackgroundAnalysisScope enhancements"#59492

Closed
RikkiGibson wants to merge 1 commit intomainfrom
revert-57172-BackgroundAnalysisScope
Closed

Revert "BackgroundAnalysisScope enhancements"#59492
RikkiGibson wants to merge 1 commit intomainfrom
revert-57172-BackgroundAnalysisScope

Conversation

@RikkiGibson
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson commented Feb 11, 2022

Reverts #57172

This PR is suspected of introducing RPS regressions.

Last known good: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/379422
First known bad: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/379438
Revert validation: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/379561

Regressed counters:

  • UWP64.ProjectManagement - RegressedBeyondThreshold / 0200.Xaml Designer Load / RefSet_Image_Delta_devenv / Regressed: 6668288.00 Bytes (8.10%)
  • WebToolsVS64.Debugging - RegressedBeyondThreshold / 0300.Stop Debugging / CLR_BytesAllocated_Total_devenv / Regressed: 193326051.20 Bytes (19.17%)
  • WinformsVS64.Designer - RegressedBeyondThreshold / 0200.Open Existing WinForm - Async / RefSet_Image_Delta_devenv / Regressed: 12410880.00 Bytes (620.27%)

Please advise whether the failures are plausibly connected with this PR, and if so, whether we should revert, or take a follow-up PR to fix, or seek an exception. @genlu @mavasani @CyrusNajmabadi @sharwell.

@ghost ghost added the Area-IDE label Feb 11, 2022
@RikkiGibson RikkiGibson marked this pull request as ready for review February 11, 2022 19:56
@RikkiGibson RikkiGibson requested review from a team as code owners February 11, 2022 19:56
@mavasani
Copy link
Copy Markdown
Contributor

I suspect the regression could be from this PR. Solution crawler is turned off in RPS, and this PR adds a code path that might have accidentally bypassed the solution crawler option check and hence led to it executing causing the regression.

@RikkiGibson I can create a simple PR to fix the above issue instead of reverting the PR. Would it be possible to validate RPS with that and if it still doesn’t fix these regression then revert the entire PR?

@RikkiGibson
Copy link
Copy Markdown
Member Author

Sounds great. Please link to this PR when you create it. Then provide your PRNumber and CommitSHA (not Commit) in this pipeline, and link to the pipeline run in your PR.

mavasani added a commit to mavasani/roslyn that referenced this pull request Feb 11, 2022
@mavasani
Copy link
Copy Markdown
Contributor

mavasani commented Feb 11, 2022

Thanks @RikkiGibson. Created #59498 to attempt to fix the RPS regression and kicked off the RPS validation PR for it.

@sharwell sharwell marked this pull request as draft February 11, 2022 23:04
@sharwell
Copy link
Copy Markdown
Contributor

Marked as draft as #59498 is preferred.

@RikkiGibson
Copy link
Copy Markdown
Member Author

Heads up that RPS is passing in the validation insertion for this revert. Doesn't change our plan for what to do but helps confirm that the problem is what we think it is.

@sharwell
Copy link
Copy Markdown
Contributor

@RikkiGibson It looks like #59498 also passed RPS, aside from an NGEN issue which is likely unrelated.

RikkiGibson pushed a commit that referenced this pull request Feb 12, 2022
@RikkiGibson RikkiGibson deleted the revert-57172-BackgroundAnalysisScope branch February 13, 2022 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants