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.

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

Look for performance opportunities #53

Open
jnm2 opened this issue Nov 7, 2019 · 7 comments
Open

Look for performance opportunities #53

jnm2 opened this issue Nov 7, 2019 · 7 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Nov 7, 2019

AnnotatorBuildTask is taking 24.6 seconds for rebuilds out of 48.3 seconds total, more than doubling the build time. I guess the first step would be to profile?

image

@JohanLarsson
Copy link

How do you generate this report?

@jnm2
Copy link
Contributor Author

jnm2 commented Nov 8, 2019

http://msbuildlog.com/. It's a bit like F12 developer tools but for MSBuild.

@jnm2
Copy link
Contributor Author

jnm2 commented Nov 9, 2019

Here's what dotTrace found. Highlights:

  • 42% of the time is Cecil writing the assembies (9.8 sec)
  • 19% of the time is Cecil trying to resolve referenced assemblies out of the GAC (4.5 sec)
  • 12% of the time is Cecil trying to resolve referenced assemblies out of a folder (2.8 sec)
  • 8.7% of the time is AnnotateMethod (2 sec)

image

Ideas

  • Use a custom assembly or type resolver that skips the GAC
  • Parallelize (separate I/O from CPU-bound work?)
  • Use hash tables instead of string comparisons to e.g. all exported types in an assembly, or instead of SingleOrDefault to find a method

@jnm2
Copy link
Contributor Author

jnm2 commented Nov 9, 2019

Cecil's assembly resolver doesn't specify deferred loading when it reads assembly files. A custom resolver might pay off.

@jnm2
Copy link
Contributor Author

jnm2 commented Nov 9, 2019

😃 huge win with #54.

Total assembly resolve time is 964 ms (version 1.0.0-alpha.99.g7d2f4a388d):

image

Where alpha.97 took 8328 ms:

image

Total time is now 17.4 seconds:

image

@sharwell
Copy link
Member

sharwell commented Nov 11, 2019

#54 is a good start. I think we could consider this issue closed with a second step modeled after dotnet/roslyn-sdk#329.

@sharwell
Copy link
Member

💡 As an expansion of #53 (comment), we could place a text file in the output directory listing the inputs to the conversion. It wouldn't be consumed directly by the build, but it could be used as the lock file and would make it easier to review the outputs manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants