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.

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

Performance improvements for formatting analyzer #32845

Merged
merged 19 commits into from Feb 9, 2019

Conversation

Projects
None yet
4 participants
@sharwell
Copy link
Member

sharwell commented Jan 26, 2019

Recommended review strategy: review each commit.

On request I can easily break up the pull request to simplify review of individual changes, but keep in mind nearly all of the observable performance improvement comes from the final changes in the PR.

Times prior to this change:

  • 46150ms
  • 46215ms
  • 47086ms

Times with IFormattingRule replaced with AbstractFormattingRule (eliminate interface dispatch):

  • 46215ms
  • 45208ms
  • 45355ms
Times with virtual dispatch and elimination of ThreadLocal<List<T>>:

Moved to #32952

  • 45618ms
  • 44991ms
  • 46964ms
Times with above plus IndentationOptions:

Change is not included in this pull request.

  • 48296ms
  • 45684ms
  • 44475ms

Times with generics expanded to per-operation non-generic types.

  • 40041ms
  • 40309ms
  • 41543ms

@sharwell sharwell added the Area-IDE label Jan 26, 2019

@sharwell sharwell requested a review from dotnet/roslyn-ide as a code owner Jan 26, 2019

Show resolved Hide resolved eng/Versions.props Outdated
@CyrusNajmabadi

This comment has been minimized.

Copy link
Contributor

CyrusNajmabadi commented Jan 27, 2019

Details on the level of perf improvement?

@sharwell

This comment has been minimized.

Copy link
Member Author

sharwell commented Jan 27, 2019

Details on the level of perf improvement?

No details yet. Need to set up test scenarios and get measurements. These changes are based on profiling a build of Roslyn.sln.

@ivanbasov
Copy link
Contributor

ivanbasov left a comment

:shipit:

@sharwell sharwell force-pushed the sharwell:format-perf branch from c853cfd to 735f9df Jan 30, 2019

Pull request was rewritten after review

@sharwell

This comment has been minimized.

Copy link
Member Author

sharwell commented Jan 30, 2019

Details on the level of perf improvement?

Added to top.

This change reduced the CPU time in the formatting code from about 308 seconds to about 260 seconds. Measured with dotnet-format -w Roslyn.sln -v detailed --dry-run.

@JoeRobich
Copy link
Member

JoeRobich left a comment

lgtm

@sharwell

This comment has been minimized.

Copy link
Member Author

sharwell commented Feb 8, 2019

@ivanbasov Can you take a look at this again?

@ivanbasov
Copy link
Contributor

ivanbasov left a comment

:shipit:

@sharwell sharwell added this to the 16.1.P1 milestone Feb 9, 2019

@sharwell sharwell merged commit d260ee0 into dotnet:master Feb 9, 2019

3 checks passed

license/cla All CLA requirements met.
Details
roslyn-CI #20190130.12 succeeded
Details
roslyn-integration-CI #20190130.12 succeeded
Details

@sharwell sharwell deleted the sharwell:format-perf branch Feb 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment