tunnelvisionlabs / ReferenceAssemblyAnnotator

IL weaver to add nullability annotations to .NET reference assemblies

MIT License

72 stars 8 forks source link

Look for performance opportunities #53

Open jnm2 opened 4 years ago

jnm2 commented 4 years ago

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?

JohanLarsson commented 4 years ago

How do you generate this report?

jnm2 commented 4 years ago

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

jnm2 commented 4 years ago

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)

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 commented 4 years ago

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

jnm2 commented 4 years ago

😃 huge win with #54.

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

Where alpha.97 took 8328 ms:

Total time is now 17.4 seconds:

sharwell commented 4 years ago

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

Place the rewritten reference assemblies in a folder under %TEMP%, such as %TEMP%/annotated-assemblies/$(hash), where $(hash) is a hash formed from the TFM, the version of the annotator package, and the package ID and version of the reference annotated assemblies. This prevents the rewriter from performing multiple rewrites during a single build, and also fixes #48.
Use a lock file to ensure files are not used concurrently by multiple processes. This would also fix #40.

sharwell commented 4 years ago

💡 As an expansion of https://github.com/tunnelvisionlabs/ReferenceAssemblyAnnotator/issues/53#issuecomment-552468534, 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.